Unnecessary pop & push

[mkustermann]

In binaryen optimized dart2wasm code I often encounter extremely redudant pop+pushes. See pop_push_issue.tar.gz:

  (func $Sub1 (param $var0 i64) (result (ref $Base))
    (local $var1 (ref $_DefaultMap&_HashFieldBase&MapMixin))
    (local $var2 (ref $_DefaultMap&_HashFieldBase&MapMixin))
    (local $var3 (ref $_DefaultMap&_HashFieldBase&MapMixin))
    (local $var4 (ref null $#Top))
    (local $var5 (ref $<Sub1.-constructor-context>))
    (local $var6 (ref $Base))
    i32.const 108
    i32.const 0
    i32.const 36
    i32.const 0
    local.get $var0
    struct.new $<Sub1.-constructor-context>
    local.tee $var5
    global.get $global2
    global.get $"C333 _FunctionType"
    struct.new $#Closure-0-0
    call $"new Base (initializer)"
    local.set $var4
    local.set $var3
    local.set $var2
    local.tee $var1
    local.get $var2
    local.get $var3
    local.get $var4
    struct.new $Base
    global.get $"C347 \"Sub1(\""
    i32.const 68
    local.get $var5
    struct.get $<Sub1.-constructor-context> $field0
    struct.new $BoxedInt
    global.get $"C17 \")\""
    call $JSStringImpl._interpolate3
    call $print
  )

The $var[1-4]s are assigned once and loaded once and poped in one order and pushed in reverse, i.e. leaving the stack unchanged. So I'd assume this

    call $"new Base (initializer)"
    local.set $var4
    local.set $var3
    local.set $var2
    local.tee $var1
    local.get $var2
    local.get $var3
    local.get $var4
    struct.new $Base

could be optimized to

    call $"new Base (initializer)"
    struct.new $Base

(if I don't misunderstand something entirely here)

The optimization was done via

% 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 -Os test.wasm -o test.OsOsOsOs.wasm

[tlively]

Looks like code we would emit when the call has a multivalue return. We would need to improve or replace StackIR to fix this, since it is impossible to represent the result of the call flowing directly into the struct.new in the main Binaryen IR. cc @stevenfontanella fyi because we were talking about multivalue problems recently.

[kripken]

I think this might be easy to fix in StackIR. Taking a look now.

[kripken]

#8623 should handle this. It saves 0.4% code size on this testcase.

[mkustermann]

Thanks!

As some background: We use multi-return-values pervasively for one particular thing: class constructors: We do that because we have to evaluate all class field initializers first, of the class and all super classes (initializers can have side-effects as well). Then the initializer function returns values for dart class fields via multi-return-values. The constructor allocator function then creates the wasm struct (+ 2 extra i32 fields: class id and identity hash code).

Doing it like this allows us to make the wasm struct for the class use immutable fields -- the alternative would be make the wasm struct fields mutable so we can allocate the struct first and make field initializer write into it - but making fields writable may hinder other binaryen and JIT optimizations

@tlively @kripken A follow-up question regarding this

a) Does binaryen's inlining heuristics take this into account? i.e. if a multi-value-return function call is inlined a bunch of tuple-extract code will disappear and make call site smaller.

b) Does binaryen have support for optimizing this in a way by moving some code around? Let's say we have

    call $"new Base (initializer)"
    local.set $var4
    local.set $var3
    local.set $var2
    local.set $var1
    i32.const 43
    i32.const 0
    local.get $var1
    local.get $var2
    local.get $var3
    local.get $var4
    struct.new $Base

Can it move the i32.const ... instructions before call and then run the simplification, i.e. turn it into

    i32.const 43
    i32.const 0
    call $"new Base (initializer)"
    struct.new $Base

[kripken]

a) Inlining uses fixed heuristics, but Monomorphization will measure the effect after inlining. I think that doesn't take into account StackIR atm, but if we find that is an issue it could be added.

b) BinaryenIR and StackIR specifically will reorder in many cases, though I'm not sure offhand if this particular case would be optimized without trying it on a concrete file.