Running `wasm-opt` with `-Os` leaves NOP / dead control flow behind

[mkustermann]

See attached flute.tar.gz

We start with an unoptimized flute.wasm file and run wasm-opt

% wasm-opt -g --enable-gc --enable-reference-types --enable-multivalue --enable-exception-handling --enable-nontrapping-float-to-int --enable-sign-ext --enable-bulk-memory --enable-threads '--no-inline=*<noInline>*' --traps-never-happen \
    -Os \
    flute.wasm \
    -o flute.Os.wasm

This -Os leaves NOP blocks behind:

 (func $_InterfaceType.toString ...
  ...
    block $label0 (result i32)
      local.get $var0
      ref.cast $_InterfaceType
      local.tee $var7
      struct.get $_InterfaceType $classId
      br $label0
    end $label0
  ...
)

It also doesn't prune branches with unreachable (some variants of this):

(func $_FunctionType.toString
  ...
            if (result (ref $JSStringImpl))
              local.get $var6
              unreachable
            else
              local.get $var6
              block $label5 (result (ref $JSStringImpl))
                local.get $var9
                i32.const 0
                global.get $"C8 DiagnosticLevel"
                ref.null none
                local.get $var9
                struct.get $_Type $field0
                i32.const 3066
                i32.add
                call_indirect (param (ref $#Top) i32 (ref $DiagnosticLevel) (ref null $TextTreeConfiguration)) (result (ref $JSStringImpl))
                br $label5
              end $label5
              call $JSStringImpl.+
            end
  ...  
)

If we run another -Os pass then those issues disappear

% wasm-opt -g --enable-gc --enable-reference-types --enable-multivalue --enable-exception-handling --enable-nontrapping-float-to-int --enable-sign-ext --enable-bulk-memory --enable-threads '--no-inline=*<noInline>*' --traps-never-happen \
    -Os \
    flute.Os.wasm \
    -o flute.Os.Os.wasm

From a users point of view, I wouldn't expect an -Os to leave these code patterns behind. We can of course run multiple -Os rounds, but that a) may do more work than needed and b) raises the question of how many -Os passes one may need to run to cleanup things left by previous -Os passes.

[kripken]

This is a current limitation. Most passes will proceed while they see more work to do, but the pass pipeline as a whole does not keep running while one pass opens up work for another. Investigating this case, that is what happens here.

We do have an optional flag --converge, that will keep running all the passes while code size decreases, but it can have unpredictable compile times (and code size isn't always the right metric).

In the long term, we may want a smarter pass manager that sees which passes open up work for others and keeps running them, but there are no immediate plans for that. Running multiple -Os etc. is the simplest workaround for now.

If this is a significant problem, perhaps improving --converge is a short-term option we can consider, but I'd hope this is only a matter of a few bytes here and there?

[tlively]

What are the passes involved in the cycle of creating new opportunities here? Perhaps we could do another targeted fix by combining passes or having them run each other as part of their internal iterations.

[kripken]

Here, removing the useless blocks only requires --remove-unused-brs --vacuum. But the reason those are the right passes is just a coincidence - there was simply so much work after inlining that it stopped at some "random" point. It could have been some entirely other set of passes needed there, in other words, and it might require cycles of them.