diff options
author | Alon Zakai <azakai@google.com> | 2021-04-15 16:18:06 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-04-15 16:18:06 -0700 |
commit | bd2e8661a31aa02f701e31110108a5f5c194afed (patch) | |
tree | 15ad40486ec605598d2f3580a580d4f5fe4fc756 /src/tools/wasm-shell.cpp | |
parent | ac2a49bc53d4b0817657aed9f89f33868e5ab3b9 (diff) | |
download | binaryen-bd2e8661a31aa02f701e31110108a5f5c194afed.tar.gz binaryen-bd2e8661a31aa02f701e31110108a5f5c194afed.tar.bz2 binaryen-bd2e8661a31aa02f701e31110108a5f5c194afed.zip |
[Wasm GC] Fix precompute on GC data (#3810)
This fixes precomputation on GC after #3803 was too optimistic.
The issue is subtle. Precompute will repeatedly evaluate expressions and
propagate their values, flowing them around, and it ignores side effects
when doing so. For example:
(block
..side effect..
(i32.const 1)
)
When we evaluate that we see there are side effects, but regardless of them
we know the value flowing out is 1. So we can propagate that value, if it is
assigned to a local and read elsewhere.
This is not valid for GC because struct.new and array.new have a "side
effect" that is noticeable in the result. Each time we call struct.new we get a
new struct with a new address, which ref.eq can distinguish. So when this
pass evaluates the same thing multiple times it will get a different result.
Also, we can't precompute a struct.get even if we know the struct, not unless
we know the reference has not escaped (where a call could modify it).
To avoid all that, do not precompute references, aside from the trivially safe ones
like nulls and function references (simple constants that are the same each time
we evaluate the expression emitting them).
precomputeExpression() had a minor bug which this fixes. It checked the type
of the expression to see if we can create a constant for it, but really it should
check the value - since (separate from this PR) we have no way to emit a
"constant" for a struct etc. Also that only matters if replaceExpression is true, that
is, if we are replacing with a constant; if we just want the value internally, we have
no limit on that.
Also add Literal support for comparing GC refs, which is used by ref.eq. Without
that tiny fix the tests here crash.
This adds a bunch of tests, many for corner cases that we don't handle (since
the PR makes us not propagate GC references). But they should be helpful
if/when we do, to avoid the mistakes in #3803
Diffstat (limited to 'src/tools/wasm-shell.cpp')
0 files changed, 0 insertions, 0 deletions