Global store-to-load forwarding

[mkustermann]

This is similar to #8016 & #8002

See this repro.tar.gz. The optimized version was obtained via open world compilation

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

It leaves the following behind

  (type $JSExternWrapper (sub $Object (struct
    (field $field0 i32)
    (field $field1 (mut i32))
    (field $_externRef externref))))

  (global $.a (import "" "a") (ref extern))
  (global $"C284 \"a\"" (ref $JSExternWrapper) (i32.const 97) (i32.const 0) (global.get $.a) (struct.new $JSExternWrapper))

  (func $"findExact <noInline>" (param $var0 (ref $JSExternWrapper)) (result i64)
    (local $var1 (ref $JSExternWrapper))
    block $label0
      block $label1
        block $label2
          block $label3 (result i32)
            block $label4
              global.get $"C284 \"a\""
              local.tee $var1
              struct.get $JSExternWrapper $field0
              i32.const 97
              i32.ne
              br_if $label4
              local.get $var0
              struct.get $JSExternWrapper $_externRef
              local.get $var1
              struct.get $JSExternWrapper $_externRef
              call $"wasm:js-string.equals (import)"
              i32.eqz
              br_if $label4
              i32.const 1
              br $label3
            end $label4
            i32.const 0
          end $label3
          i32.eqz

Notice that the

• $C284 global is a non mutable global with known values being initialized
• $JSExternWrapper.$field0 is a non mutable i32 field
  Therefore the

              global.get $"C284 \"a\""
              local.tee $var1
              struct.get $JSExternWrapper $field0

should be turned into

              i32.const 97

and the

              local.get $var1
              struct.get $JSExternWrapper $_externRef

should be turned into

             global.get $.a

[kripken]

The missing thing here is to analyze the reference through locals, which GSI does not do.

But this is already done by the GUFA pass. I tried replacing -Os -Os -Os with -Os --gufa -Os -Os and it optimizes properly.

GUFA is a slower pass, but running it after -Os may be fast enough, and could give you many other benefits?

[mkustermann]

I was under the impression that --gufa requires --closed-world. It does seem indeed one can run wasm-opt with --gufa but without the --closed-world flag.

@kripken Is it safe to do so? e.g. here it's mentioned:

GUFA atm assumes a closed world. I see we don't error without setting --closed-world, which is a bug

From this it sounds like it may not be safe.

If it's actually safe to do, it may be good to update the documentation/wiki about GUFA.
If it's not safe to do, it would be very good if the compiler would error, to avoid silently misoptimizing modules.

Though as mentioned in #8016 as well, these kind of code patterns I would expect to be optimized without expensive global / whole-world analysis phases. Is it because the local optimizations an -Os performs do not include any flow analysis on a per-function level? Is the issue in #8641 similar (in that re-using a local prevents optimizations)?

[kripken]

GUFA should be safe atm, and is fuzzed in open world. Thanks for the link to #5751 - that was before we improved GUFA. I closed it now after verifying.

Though as mentioned in #8016 as well, these kind of code patterns I would expect to be optimized without expensive global / whole-world analysis phases. Is it because the local optimizations an -Os performs do not include any flow analysis on a per-function level? Is the issue in #8641 similar (in that re-using a local prevents optimizations)?

Ideally yes, you are absolutely right. This could be done in GSI like in those issues. However, GSI just does not track local flow atm. Adding it would have downsides (complexity and overhead).

Other passes than GSI do flow analysis, but are not powerful enough to help here.

Overall I think it might be simpler to just use GUFA. If we find bugs or overhead in GUFA, fixing them might be the best path here.

[mkustermann]

We'll enable the --gufa flag then. Thanks, @kripken !

Consider updating the wiki saying that even though GUFA is a global optimization, it doesn't require closed world assumptions.

[mkustermann]

This seems to reveal bugs in binaryen. A few tests are failing on the CI run. For one of them I've made a repro & filed: #8718

[kripken]

Thanks, I updated the wiki.

Also thanks for the bugreport - makes sense that we might find problems on real-world code here, though I was hoping the fuzzer had found them all already...