Regression on optimized module size depending on final markers

[askeksa]

The commit https://github.com/WebAssembly/binaryen/commit/7df30640820c9b4acfc69ffc7616809a727d1241 introduced a severe size regression for fs.wasm.gz when optimized with

wasm-opt -all --closed-world -tnh -O3 --type-ssa --gufa -O3 --type-merging -O1

Before the change, it optimized to 1520024 bytes. After, it optimizes to 1890468 bytes.

The module declares all struct types as non-final. If it instead marks all types final that don't have any subtypes (ff.wasm.gz), the change introduces a smaller regression from 1540124 to 1639628 bytes.

[askeksa]

With recent changes, the regression seems to have disappeared.

[tlively]

🤔

Did it actually disappear or was it just masked by other code size wins? Would you be able to bisect to find the fixing commit?

[askeksa]

On closer look, it wasn't fixed, actually. If I add --type-finalizing at the end, the size is good again, but without it, it's still bad. The main difference between the modules is that the number of types is reduced from about 22k to 3k. So apparently --type-merging doesn't quite eliminate types as expected.

[kripken]

@askeksa is it possible for you to find some example types that are not merged but should be?

[tlively]

Also, are you running --remove-unused-types at the end of your pipeline? If not, that would explain the regression since type merging adds a bunch of new types and makes the old types unused without actually removing them on its own.

[askeksa]

Well, that's exactly it. Apparently, before https://github.com/WebAssembly/binaryen/commit/7df30640820c9b4acfc69ffc7616809a727d1241, --type-merging was removing the unused types by itself, whereas now it's not. They are removed by --type-finalizing, so that's how it works out for us now.

IMO, it would seem natural for --type-merging to remove all the types it orphans, but in any case it's now clear what is going on.

[kripken]

Thanks @askeksa , now I see. To confirm, I ran the testcase from the issue description and saw that adding --remove-unused-types (or --type-finalizing, or both, doesn't matter) shrinks the output from 1568585 to 1533965 bytes.

Looking into this, the issue is the final --type-merging -O1 at the end. --type-merging does actually remove unused types (it calls TypeMapper, which uses GlobalTypeRewriter, which is how --remove-unused-types works). What seems to happen here is that -O1 actually makes a few more types unused inside private rec groups, just by the simple things that -O1 does happening to remove the last references to certain types. But -O1 does not run --remove-unused-types internally, only -O2 and above do.

That is, to fix this, replace --type-merging -O1 with --type-merging -O2 or higher (or, --type-merging -O1 --remove-unused-types, but there might be other things -O1 misses here).

I think this is working as intended, since -O1 is meant to be a fast subset of passes, not a comprehensive optimization pipeline.

[tlively]

Every type optimization pass removes types that were unused before the pass, but can produce new unused types. --remove-unused-types is basically a no-op type optimization pass, so it's only effect is to remove the previously unused types. Should we add --remove-unused-types to -O1?

[kripken]

We could, but in general we want to keep -O1 minimal. It's not meant as a production-grade setting, as aside from leaving unused types it can also leave unused locals and other things. So I'd prefer to not add it, but if this happens to be very useful I also wouldn't object, as we don't have a clear rule for what to add.