[Custom Descriptors] Optimize away unneeded descriptors in GlobalTypeOptimization #7872

kripken · 2025-08-29T17:25:53Z

This adds descriptor support to struct-utils.h as an extra field. It is
reported as index "-1" when an index is used in the API, allowing
passes to skip it where irrelevant.

That new support is then used in a single pass, GTO, where we just
see if a descriptor can be removed, and remove it if so.

Diff without whitespace is smaller.

kripken · 2025-08-29T18:13:29Z

Oh, the fuzzer found a subtyping issue here. Looking into it...

tlively · 2025-08-29T18:16:39Z

src/ir/struct-utils.h

@@ -320,6 +362,10 @@ template<typename T> class TypeHierarchyPropagator {
              work.push(*superType);
            }
          }
+          // Propagate the descriptor.
+          if (superInfos.desc.combine(infos.desc)) {


We probably need to check whether the supertype actually has a descriptor first.

This is safe either way: if there is no descriptor, there is nothing to propagate.

Given the propagation we do is typically copying a few booleans, I'm not sure if adding a check would make things faster.

I'm not so worried about speed, but rather modeling things accurately. It would be confusing to find during debugging that a type with no descriptor has descriptor information. One option would be to put the desc info in a std::optional, but I suppose that users of this utility will have their own default "bottom" information that works just as well.

Ah, I see. Yeah, maybe a std::optional would make debugging clearer. But that would add overhead and code clutter. I added a comment, maybe that's good enough?

tlively · 2025-08-29T18:17:52Z

src/ir/struct-utils.h

@@ -333,6 +379,10 @@ template<typename T> class TypeHierarchyPropagator {
              work.push(subType);
            }
          }
+          // Propagate the descriptor.
+          if (subInfos.desc.combine(infos.desc)) {


And for both supertypes and subtypes we can probably skip combining descriptor values if the original type does not have a descriptor.

src/passes/ConstantFieldPropagation.cpp

src/passes/GlobalTypeOptimization.cpp

tlively · 2025-08-29T18:47:08Z

src/passes/GlobalTypeOptimization.cpp

+        // To remove a descriptor, it must not be used in either subtypes or
+        // supertypes, to not break validation. It must also have no write (see


We should still be able to remove a descriptor even if subtypes need to keep their descriptors, as long as we can remove the descriptors from all the supertypes.

It would also be great if we could take reference exactness into account and avoid propagating info to supertypes and subtypes when we don't need to.

Maybe I'm misreading you, but are you suggesting this should work?

(module (rec (type $A (sub (descriptor $A.desc (struct)))) (type $A.desc (sub (describes $A (struct )))) (type $B (sub $A (descriptor $B.desc (struct)))) (type $B.desc (sub (describes $B (struct)))) ) (func $test (drop (struct.new $A (struct.new $A.desc) ) ) (drop (ref.get_desc $B ;; force $B's descriptor to remain (struct.new $B (struct.new $B.desc) ) ) ) ) )

Here there is a subtype of $A that needs a descriptor, but $A itself does not. But this does not validate: $B.desc must be a subtype of $A.desc. And, once that is added, we cannot optimize, as the descriptor subtyping blocks us.

$B.desc only needs to be a subtype of $A's descriptor if $A has a descriptor. This is perfectly valid:

(rec (type $A (sub (struct))) (type $B (sub $A (descriptor $B.desc (struct)))) (type $B.desc (sub (describes $B (struct)))) )

It looks like you removed $A.desc and $B.desc's subtyping of it. Yeah, that should work, but only if we unsubtype here at the same time? I'm not sure if we want this pass to do that.

Oh right. But then Unsubtyping won't be able to do this either because it doesn't modify descriptor/describes relationships.

One option would be to handle unneeded descriptors that need to remain descriptors for validation purposes by giving them a trivial new type to describe. That would let the original described type be optimized and allow future optimization passes to clean up the descriptor type and the trivial new described type.

If the program requires $B <: $A and requires that $B.desc remain the descriptor of $B, then unsubtyping won't be able to optimize anything because $B <: $A implies $B.desc <: $A.desc. Then GTO won't be able to do anything either because keeping $B.desc as $B's descriptor forces keeping $A.desc as $A's descriptor with the current code. If we added the trivial described types, then --gto --unsubtyping would be able to fully optimize this case as well.

I see, thanks. Makes sense.

This would need the new trivial described type to still be a supertype of the sub-descriptor, correct? It seems tricky to get that right, especially if there are other uses (say, casts) on the descriptor types...

Let's leave all this for a TODO perhaps?

No, the new trivial described types shouldn't need to have any supertypes or subtypes. Leaving this as a TODO sounds good, although some of the new propagation infrastructure wouldn't be necessary if we did this.

Hmm, please see the commit before last entitled tlively's idea? - does that do what you were thinking?

It doesn't quite work - some bug with temp types deep in the TypeBuilder - but I posted it because I don't see how

some of the new propagation infrastructure wouldn't be necessary

What code should have been deleted? This seems to just add code.

I guess you still need to propagate needed up and down the descriptor hierarchy to determine which types need trivial described types, so I was wrong about being able to delete code :(

See separate comments on the implementation of the idea.

test/lit/passes/gto-desc.wast

tlively · 2025-08-29T19:07:51Z

test/lit/passes/gto-desc.wast

+  )
+)
+
+;; As above, but use $B's. This also stops everything.


We should be able to optimize $A here!

(same issue as above with my wat example, I think)

kripken · 2025-08-29T19:56:22Z

(bug fixed where descriptors subtyped but not the main types)

tlively · 2025-08-29T20:46:04Z

src/passes/GlobalTypeOptimization.cpp

+      }
+
+      // Propagate.
+      descPropagator.propagateToSuperAndSubTypes(map);


Ah, propagating descriptor info up to types without descriptors actually produces worse results here. If two different types have descriptors and they share a supertype without a descriptor, we should not propagate constraints on one descriptor to the other.

(type $super (sub (struct))) (type $A (sub $super (descriptor $A.desc (struct)))) (type $A.desc (describes $A (struct))) (type $B (sub $super (descriptor $B.desc (struct)))) (type $B.desc (describes $B (struct)))

Here propagateToSuperAndSubTypes will currently propagate descriptor information from $A to $B and vice versa. But that's overly conservative.

(Actually it doesn't matter in this particular case because the property being propagated is not tied to the descriptor field, but I guess this reminded me of the issue.)

Hmm, this is descPropagator: it propagates along descriptors. In your example, the describees are where there is subtyping.

And, for the descriptors, I think this problem can't happen? Doesn't a supertype of a descriptor need to be a descriptor?

Looks like our last comments here raced. Are you saying this might happen elsewhere?

Right, I realized after I wrote the opening comment that there's no problem with this particular code. But e.g. using $A.desc should not force $B.desc to be kept around.

Thanks, yes, now I see. Fixed in the last commit: now that we have proper descriptor subtype propagation (in the code after it in the source), we can propagate describees only to subtypes, allowing that optimization.

We should still fix the propagator not to propagate descriptor information through types that don't have descriptors, though.

Done in d0ec85a

I don't quite see how to test that, though, but it is at least good for efficiency.

This might become more important in CFP?

Hmm, yeah, other passes might notice, good point.

This reverts commit 57f120c.

test/lit/passes/gto-desc.wast

tlively · 2025-08-30T00:20:14Z

src/passes/GlobalTypeOptimization.cpp

+      }
+
+      // Propagate.
+      descPropagator.propagateToSuperAndSubTypes(map);


We should still fix the propagator not to propagate descriptor information through types that don't have descriptors, though.

tlively

Comments on tlively's idea, let's see how GitHub renders them...

tlively · 2025-08-30T00:27:57Z

src/passes/GlobalTypeOptimization.cpp

+            // that, we add a new type A2 for A.desc to describe, which keeps
+            // the property that A.desc and B.desc are a parent/child pair of
+            // descriptors, which is necessary for validation.
+            haveUnneededDescriptors[*described] = HeapType(described->getStruct());


No need to copy $A's contents. We can always just use trivial empty structs (which means that haveUnneededDescriptors can remain a set).

tlively · 2025-08-30T00:30:41Z

src/passes/GlobalTypeOptimization.cpp

+            if (value) {
+              value = getTempHeapType(*value);
+            }
+            typeBuilder.setDescribed(i, value);


This isn't quite right. You need to grow the type builder to make space for the new described type, put an empty struct in that new last entry of the type builder, set it to be described by the current type, and set the current type to describe the new type.

Makes sense - this is definitely not as simple as I'd hoped. We need to add some kind of new hook in TypeRewriter that lets the user grow the TypeBuilder, and also to somehow set up the right order (we can't append the described type after the descriptor).

I added a TODO for this.

Oh right, that ordering makes it extra tricky...

Co-authored-by: Thomas Lively <[email protected]>

tlively · 2025-09-02T17:44:31Z

src/ir/struct-utils.h

+  // combine() a non-existent descriptor, we are doing unneeded work, but the
+  // data here is typically just a few bools, so it is simpler and likely
+  // faster to just copy those rather than check if the type has a descriptor.)


We no longer combine when the descriptor does not exist.

TypeHierarchyPropagator doesn't do so below, that's correct, but StructValuesMap still does.

src/ir/struct-utils.h

Co-authored-by: Thomas Lively <[email protected]>

kripken added 30 commits August 28, 2025 08:40

work

56d5165

work

440016f

work

bdf1045

work

1e24c1d

work

1e958b5

work

c3ee741

prep

6086dc9

work

7ac2730

work

b826d63

work

427297f

work

a2c4914

work

991a129

work

b457ed2

work

5b25a96

work

b91aef6

work

a5697f3

work

48c66ca

work

e002c61

work

5be8c21

work

33e9e87

work

3ed905a

work

7b205cc

work

b62a6ec

work

98aa913

work

c6f45fe

work

4f05e6f

work

1e4d63f

work

757e50b

work

42f00fc

format

9075189

work

0a4dc43

tlively reviewed Aug 29, 2025

View reviewed changes

kripken added 4 commits August 29, 2025 12:48

work

90f1fb5

work

6b722ef

work

9e1409a

work

c774441

kripken added 4 commits August 29, 2025 12:59

feedback: comments

08cc60b

feedback: remove parallel sets

a490a8e

feedback: test field names

88340c3

feedback: comment

88cc7dc

tlively reviewed Aug 29, 2025

View reviewed changes

kripken added 4 commits August 29, 2025 15:39

tlively's idea?

57f120c

Revert "tlively's idea?"

8a4286a

This reverts commit 57f120c.

Optimize siblings

567ecbd

fix nested global effects

2950719

tlively reviewed Aug 30, 2025

View reviewed changes

kripken and others added 7 commits September 2, 2025 09:48

fix

03ff3e5

fix casts+test

55e7065

test

d406990

Update test/lit/passes/gto-desc.wast

b240e30

Co-authored-by: Thomas Lively <[email protected]>

Do not propagate a descriptor to a super without one

d0ec85a

Merge remote-tracking branch 'myself/gto.desc' into gto.desc

276a188

TODO

46634b3

tlively approved these changes Sep 2, 2025

View reviewed changes

Update src/ir/struct-utils.h

eb266ee

Co-authored-by: Thomas Lively <[email protected]>

kripken merged commit 7ac88c4 into WebAssembly:main Sep 2, 2025
16 checks passed

kripken deleted the gto.desc branch September 2, 2025 20:19

		// To remove a descriptor, it must not be used in either subtypes or
		// supertypes, to not break validation. It must also have no write (see

[Custom Descriptors] Optimize away unneeded descriptors in GlobalTypeOptimization #7872

[Custom Descriptors] Optimize away unneeded descriptors in GlobalTypeOptimization #7872

Uh oh!

Conversation

kripken commented Aug 29, 2025

Uh oh!

kripken commented Aug 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kripken Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kripken commented Aug 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tlively left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

kripken Aug 29, 2025 •

edited

Loading