| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
As suggested in
https://github.com/WebAssembly/binaryen/pull/3955#issuecomment-871016647
This applies commandline features first. If the features section is present, and
disallows some of them, then we warn. Otherwise, the features can combine
(for example, a wasm may enable feature X because it has to use it, and a user
can simply add the flag for feature Y if they want the optimizer to try to use it;
both flags will then be enabled).
This is important because in some cases we need to know the features before
parsing the wasm, in the case that the wasm does not use the features section.
In particular, non-nullable GC locals have an effect during parsing. (Typed
function references also does, but we found a way to apply its effect all the time,
that is, always use the refined type, and that happened to not break the case
where the feature is disabled - but such a workaround is not possible with
non-nullable locals.)
To make this less error-prone, add a FeatureSet input as a parameter to
WasmBinaryBuilder. That is, when building a module, we must give it the
features to use while doing so.
This will unblock #3955 . That PR will also add a test for the actual usage
of a feature during loading (the test can only be added there, after that PR
unbreaks things).
|
| |
|
|
|
|
|
|
|
|
|
| |
We have seen some cases in both Chrome and Firefox where
extremely large modules cause overhead,
#3730 (comment) (and link therein)
emscripten-core/emscripten#13899 (comment)
There is no "right" value to use as a limit here, but pick an
arbitrary one that is very high. (This setting is verified to have
no effect on the emscripten benchmark suite.)
|
| |
|
|
|
|
|
|
|
| |
When using nominal types, func.ref of two functions with identical signatures
but different HeapTypes will yield different types. To preserve these semantics,
Functions need to track their HeapTypes, not just their Signatures.
This PR replaces the Signature field in Function with a HeapType field and adds
new utility methods to make it almost as simple to update and query the function
HeapType as it was to update and query the Function Signature.
|
| |
|
|
|
|
|
|
|
|
| |
That traversal did not mention BrOn, which led to it doing incorrect work in
SimplifyLocals.
Also add assertions at the end, that aim to prevent future issues.
The rest of the fix is to make SimplifyLocals not assume that things
are a Switch if they are not an If/Block/etc., so that we don't crash
on a BrOn.
|
| | |
|
| |
|
|
|
|
|
|
|
| |
PostEmscripten will turn an invoke of a constant function
pointer index into a direct call. However, due to UB it is possible to
have invalid function pointers, and we should not crash on that
(and do nothing to optimize, of course).
Mostly whitespace; to avoid deep nesting, I added more
early returns.
|
| |
|
|
|
|
|
|
| |
This attribute is always 0 and reserved for future use. In Binayren's
unofficial text format we were writing this field as `(attr 0)`, but we
have recently come to the conclusion that this is not necessary.
Relevant discussion:
https://github.com/WebAssembly/exception-handling/pull/160#discussion_r653254680
|
| |
|
|
|
|
| |
We don't need to assign a zero value for such locals (and we can't, as no
default value exists for them).
Fixes #3944
|
| |
|
|
|
|
|
|
|
|
|
| |
We recently decided to change 'event' to 'tag', and to 'event section'
to 'tag section', out of the rationale that the section contains a
generalized tag that references a type, which may be used for something
other than exceptions, and the name 'event' can be confusing in the web
context.
See
- https://github.com/WebAssembly/exception-handling/issues/159#issuecomment-857910130
- https://github.com/WebAssembly/exception-handling/pull/161
|
| |
|
|
|
|
|
|
|
|
| |
RedundantSetElimination checks if a local already has the default
value when we assign the default to it. For a non-nullable local, however,
there is no initial value - it cannot be used before it is assigned to. So
we just need to skip such locals, and not assume they contain a default
value we can compare against (we would assert on trying to create a
"zero" for such a non-nullable type).
Fixes #3942
|
| |
|
|
|
|
|
|
| |
This is a useful alternative to extract-function when you don't know the
function's name.
Also moves the extract-function tests to be lit tests and re-uses them as
extract-function-index tests.
|
| |
|
|
|
|
|
|
|
|
| |
This is the same as rtt.sub, but creates a "new" rtt each time. See
https://docs.google.com/document/d/1DklC3qVuOdLHSXB5UXghM_syCh-4cMinQ50ICiXnK3Q/edit#
The old Literal implementation of rtts becomes a little more complex here,
as it was designed for the original spec where only structure matters. It may
be worth a complete redesign there, but for now as the spec is in flux I think
the approach here is good enough.
|
| |
|
|
|
|
|
| |
Adds a `--nominal` option to switch the type machinery from equirecursive to
nominal. Implements binary and text parsing and emitting of nominal types using
new type constructor opcodes and an `(extends $super)` text syntax extension.
When not in nominal mode, these extensions will still be parsed but will not
have any effect and will not be used when emitting.
|
| |
|
|
|
|
| |
They are basically the flip versions. The only interesting part in the impl is that their
returned typed and sent types are different.
Spec: https://docs.google.com/document/d/1DklC3qVuOdLHSXB5UXghM_syCh-4cMinQ50ICiXnK3Q/edit
|
| |
|
|
| |
Simplifies the public API to not unnecessarily take an index and simplifies the
implementation to use a single integer as state rather than a vector of indices.
|
| |
|
|
|
|
|
|
| |
Spec for it is here:
https://docs.google.com/document/d/1DklC3qVuOdLHSXB5UXghM_syCh-4cMinQ50ICiXnK3Q/edit#
Also reorder some things in wasm.h that were not in the canonical order (that has
no effect, but it is confusing to read).
|
| |
|
|
|
|
|
|
| |
Imported functions come first when modules are emitted, so to ensure the
function indices are correct, they need to come first in the symbol maps. We
never noticed this bug before because imported functions are always the first
functions when a module is parsed, so the bug never mattered in practice.
However, wasm-split adds new imported functions after parsing and these were
causing the symbol map indices to be incorrect.
|
| |
|
| |
Fixes #3895
|
| |
|
|
|
|
|
|
|
| |
As found in #3682, the current implementation of type ordering is not correct,
and although the immediate issue would be easy to fix, I don't think the current
intended comparison algorithm is correct in the first place. Rather than try to
switch to using a correct algorithm (which I am not sure I know how to
implement, although I have an idea) this PR removes Type ordering entirely. In
places that used Type ordering with std::set or std::map because they require
deterministic iteration order, this PR uses InsertOrdered{Set,Map} instead.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously we would try to stop using the allocation as much as possible,
for example not writing it to locals any more, and leaving it to other passes
to actually remove it (and remove gets of those locals etc.). This seemed
simpler and more modular, but does not actually work in some cases as
the fuzzer has found. Specifically, if we stop writing our allocation to
locals, then if we do a (ref.as_non_null (local.get ..)) of that, then we
will trap on the null present in the local.
Instead, this changes our rewriting to do slightly more work, but it is
simpler in the end. We replace the allocation with a null, and replace
all the places that use it accordingly, for example, updating types to be
nullable, and removing RefAsNonNulls, etc. This literally gets rid of the
allocation and all the places it flows to (leaving less for other passes to
do later).
|
| |
|
|
| |
Similar to struct operations, if the reference is unreachable then we do
not know the heap type, and cannot print the full expression.
|
| |
|
|
|
|
|
|
|
|
|
| |
If we run a pass that removes DWARF followed by one that could destroy it, then
there is no possible problem - there is nothing left to destroy. We can run the later
pass with no issues (and no warnings).
Also add an assertion on running a pass runner only once. That has always been
the assumption, and now that we track whether the added passes remove debug
info, we need to check it.
Fixes emscripten-core/emscripten#14161
|
| |
|
|
|
|
|
|
|
|
| |
wasm-as supports --symbolmap=FOO as an argument. We got a request to
support the same in wasm-opt. wasm-opt does have --print-function-map which
does the same, but as a pass. To unify them, use the new pass arg sugar from
#3882 which allows us to add a --symbolmap pass whose argument can be
set as --symbolmap=FOO. That perfectly matches the wasm-as notation.
For now, keep the old --print-function-map notation as well, to not break
emscripten. After we remove it there we can remove it here.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We have --pass-arg that allows sending an argument to a pass, like this:
wasm-opt --do-stuff --pass-arg=do-stuff@FUNCTION_NAME
With this PR that is equivalent to this:
wasm-opt --do-stuff=FUNCTION_NAME
That is,one can just give an argument to a pass on the commandline.
This fixes the Optional mode in command-line.h/cpp. That was not
actually used anywhere before this PR.
Also rename --extract-function's pass argument to match it. That is, the usage
used to be
wasm-opt --extract-function --pass-arg=extract@FUNCTION_NAME
Note how the pass name differed from the pass-arg name. This changes it to
match. This is a breaking change, but I doubt this is used enough to justify
any deprecation / backwards compatibility effort, and any usage is almost
certainly manual, and with PR writing it manually becomes easier as one
can do
wasm-opt --extract-function=FUNCTION_NAME
The existing test for that is kept (&renamed), and a new test added to test the
new notation.
This is a step towards unifying the symbol map functionality between
wasm-as and wasm-opt (later PRs will turn the symbol mapping pass into
a pass that receives an argument).
|
| |
|
|
|
| |
Without this fix we can segfault, as it has no body.
Fixes #3879
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If we branch to a block, and there are no other branches or a final
value on the block either, then there is no mixing, and we may be able
to optimize the allocation. Before this PR, all branches stopped us.
To do this, add some helpers in BranchUtils.
The main flow logic in Heap2Local used to stop when we reached a
child for the second time. With branches, however, a child can flow both to
its immediate parent, and to branch targets, and so the proper thing to look
at is when we reach a parent for the second time (which would definitely
indicate mixing).
Tests are added for the new functionality. Note that some existing tests
already covered some things we should not optimize, and so no tests
were needed for them. The existing ones are: $get-through-block,
$branch-to-block.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If we allocate some GC data, and do not let the reference escape, then we can
replace the allocation with locals, one local for each field in the allocation
basically. This avoids the allocation, and also allows us to optimize the locals
further.
On the Dart DeltaBlue benchmark, this is a 24% speedup (making it faster than
the JS version, incidentially), and also a 6% reduction in code size.
The tests are not the best way to show what this does, as the pass assumes
other passes will clean up after. Here is an example to clarify. First, in pseudocode:
ref = new Int(42)
do {
ref.set(ref.get() + 1)
} while (import(ref.get())
That is, we allocate an int on the heap and use it as a counter. Unnecessarily,
as it could be a normal int on the stack.
Wat:
(module
;; A boxed integer: an entire struct just to hold an int.
(type $boxed-int (struct (field (mut i32))))
(import "env" "import" (func $import (param i32) (result i32)))
(func "example"
(local $ref (ref null $boxed-int))
;; Allocate a boxed integer of 42 and save the reference to it.
(local.set $ref
(struct.new_with_rtt $boxed-int
(i32.const 42)
(rtt.canon $boxed-int)
)
)
;; Increment the integer in a loop, looking for some condition.
(loop $loop
(struct.set $boxed-int 0
(local.get $ref)
(i32.add
(struct.get $boxed-int 0
(local.get $ref)
)
(i32.const 1)
)
)
(br_if $loop
(call $import
(struct.get $boxed-int 0
(local.get $ref)
)
)
)
)
)
)
Before this pass, the optimizer could do essentially nothing with this.
Even with this pass, running -O1 has no effect, as the pass is only
used in -O2+. However, running --heap2local -O1 leads to this:
(func $0
(local $0 i32)
(local.set $0
(i32.const 42)
)
(loop $loop
(br_if $loop
(call $import
(local.tee $0
(i32.add
(local.get $0)
(i32.const 1)
)
)
)
)
)
)
All the GC heap operations have been removed, and we just
have a plain int now, allowing a bunch of other opts to run. That
output is basically the optimal code, I think.
|
| |
|
|
|
|
| |
If we can't emit something, and instead emit a replacement for it (as is
the case for a StructSet with an unreachable RTT, so we have no known
heap type for it), add a comment that mentions it is a replacement. This
might avoid confusion while debugging.
|
| |
|
|
|
|
|
|
|
| |
Instead, run RemoveUnusedModuleElements, which does that sort of thing. That
is, this pass just "extracts" the function by turning all others into imports, and then
they should almost all be removable via RemoveUnusedModuleElements, depending
on whether they are used in the table or not, whether the extracted function calls
them, etc.
Without this, we would error if a function was in the table, and so this fixes #3876
|
| |
|
|
|
|
| |
Also fix printing of unreachable StructSets, which must handle the case
of an unreachable reference, which means we do not know the RTT,
and so we must print a replacement for the StructSet somehow. Emit a
block with drops, fixing the old behavior which was missing the drops.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Precompute not only computes values, but looks at the fallthrough,
(local.set 0
(block
..stuff we can ignore..
;; the fallthrough we care about - if a value is set to local 0, it is this
(i32.const 10)
)
)
Normally that is fine, but the fuzzer found a case where it is not: RefCast may
return a different type than the fallthrough, even an incompatible type if we
try to do something bad like cast a function to a struct. As we may then
propagate the value to a place that expects the proper type, this can cause an
error.
To fix this, check if the precomputed value is a proper subtype. If it is not,
then do not look through into the fallthrough, but compute the entire thing.
(In the case of a bad RefCast of a func to a struct, it would then indicate a
trap happens, and we would not precompute the value.)
|
| |
|
|
|
|
|
| |
Some passes need setInfluences but not getInfluences, but were
computing them nonetheless.
This makes e.g. MergeLocals 12% faster. It will also help use LocalGraph
in new passes with less worries about speed.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
For example,
(if (result i32)
(local.get $x)
(return
(local.get $y)
)
(return
(local.get $z)
)
)
If we move the returns outside we'd become unreachable, but we should
not make such type changes in this pass (they are handled by DCE and
Vacuum).
(found by the fuzzer)
|
| | |
|
| |
|
|
|
|
|
| |
We tried to ignore unreachable code, but only checked the type of
the entire node. But an arm might be unreachable, and after moving
code around that requires more work to update the type. But such
cases are best left to DCE anyhow, so just check for any unreachability
and stop there.
|
| |
|
|
| |
We could more carefully see when a local is not needed there, but atm
we always add one, and that doesn't work for something nondefaultable.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Effects are fine in the moved code, if we are doing so on an if
(which runs just one arm anyhow).
Allow unreachable, which lets us hoist returns for example.
Allow none, which lets us hoist drop and call for example. For
this we also need to be careful with subtyping, as at least drop
is polymorphic, so the child types may not have an LUB (see
example in code).
Adds a small ShallowEffectAnalyzer child of EffectAnalyzer that
calls visit to just do a shallow analysis (instead of walk which
walks the children).
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
(select
(foo
(X)
)
(foo
(Y)
)
(condition)
)
=>
(foo
(select
(X)
(Y)
(condition)
)
)
To make this simpler, refactor optimizeTernary to be templated.
|
| |
|
|
|
|
|
|
|
|
|
| |
We used to print active element segments right after corresponding
tables, and passive segments came after those. We didn't print internal
segment names, and empty segments weren't being printed at all. This
meant that there was no way for instructions to refer to those table
segments after round tripping.
This will fix those issues by printing segments in the order they were
defined, including segment names when necessary and not omitting
empty segments anymore.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The existing restructuring code could turn a block+br_if into an if in
simple cases, but it had some TODOs that I noticed were helpful on
GC benchmarks.
One limitation was that we need to reorder the condition and the value,
(block
(br_if
(value)
(condition)
)
(...)
)
=>
(if
(condition)
(value)
(...)
)
The old code checked for side effects in the condition. But it is ok for it
to have side effects if they can be reordered with the value (for example,
if the value is a constant then it definitely does not care about side effects
in the condition).
The other missing TODO is to use a select when we can't use an if:
(block
(drop
(br_if
(value)
(condition)
)
)
(...)
)
=>
(select
(value)
(...)
(condition)
)
In this case we do not reorder the condition and the value, but we do
reorder the condition with the rest of the block.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
(select
(i32.eqz (X))
(i32.const 0|1)
(Y)
)
=>
(i32.eqz
(select
(X)
(i32.const 1|0)
(Y)
)
)
This is beneficial as the eqz may be folded into something on the outside. I
see this pattern in real-world code, both a GC benchmark (which is why I
noticed it) and it shrinks code size by tiny amounts on the emscripten
benchmark suite as well.
|
| |
|
|
| |
In both cases doing the ref.as_non_null last is beneficial as we have
optimizations that can remove it based on where it is consumed.
|
| |
|
|
|
| |
* Note that ref.cast has a fallthrough value.
* Optimize ref.eq on identical inputs.
|
| |
|
|
|
|
|
|
|
| |
This prevents used imports which also happen to have duplicate names and
therefore cannot be provided by wasm (JS is happen to fill these in with
polymorphic JS functions).
I noticed this when working on emscripten and directly hooking modules
together. I was seeing failures, but not in release builds (because
wasm-opt would mop these up in release builds).
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
| |
Inlined parameters become locals, and rtts cannot be handled as locals, unlike
non-nullable values which we can at least fix up. So do not inline functions with
rtt params.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
The precompute pass ignored all reference types, but that was overly
pessimistic: we can precompute some of them, namely a null and a
reference to a function are fully precomputable, etc.
To allow that to work, add missing integration in getFallthrough as well.
With this, we can precompute quite a lot of field accesses in the existing
-Oz testcase, as can be seen from the output. That testcase runs
--fuzz-exec so it prints out all those logged values, proving they have
not changed.
|
| |
|
|
|
|
| |
ref.as_non_null is not needed if the value flows into a place that traps
on null anyhow. We replace a trap on one instruction with a trap on
another, but we allow such things (and even changing trap types, which
does not happen here).
|
| |
|
|
|
|
|
|
|
| |
Renames the SIMD instructions
* LoadExtSVec8x8ToVecI16x8 -> Load8x8SVec128
* LoadExtUVec8x8ToVecI16x8 -> Load8x8UVec128
* LoadExtSVec16x4ToVecI32x4 -> Load16x4SVec128
* LoadExtUVec16x4ToVecI32x4 -> Load16x4UVec128
* LoadExtSVec32x2ToVecI64x2 -> Load32x2SVec128
* LoadExtUVec32x2ToVecI64x2 -> Load32x2UVec128
|
