| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
| |
And use it to port the very simple untee test.
|
| |
|
|
|
|
|
|
|
| |
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 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
|
| |
|
|
|
|
|
|
| |
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.
|
| |
|
|
| |
This converts most EH tests in test/passes into test/lit/passes. Fixed
some files to follow 2-space indentation and improved some comments.
|
| |
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
| |
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).
|
| |
|
|
|
|
|
|
|
|
| |
`Walker::TaskFunc` has changed from a function pointer to
`std::function` in #3494, mainly to make the EH support for `CFGWalker`
easier. We didn't notice much performance difference then, but it was
recently reported that it creased binaryen.js code size and performance.
This changes `Walker::TaskFunc` back to a function pointer and does a
little more work to manage catch index in `CFGWalker` side.
Hopefully fixes #3857.
|
| |
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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 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.
|
| |
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
|
| |
The logic there would construct the cast value separately for functions and data
(as we must), and then in an attempt to share code, would then check if the
cast succeed or not (and if not, do nothing with the cast value).
But this was wrong, as in some weird casts (like a struct to a function) we
cannot construct a valid cast value, and we error there. Instead, check if the
cast works first, once we know enough to do so, and only then construct the
cast value if so.
|
| |
|
|
|
|
| |
We truncated and extended packed values in get and set, but
not during initialization.
Found by the fuzzer.
|
| |
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
|
| |
Host limitations are arbitrary and can be modified by optimizations, so
ignore them. For example, if the optimizer removes allocations then a
host limit on an allocation error may vanish. Or, an optimization that
removes recursion and replaces it with a loop may avoid a host limit
on call depth (that is not done currently, but might some day).
This removes a class of annoying false positives in the fuzzer.
|
| |
|
|
|
|
|
|
|
|
| |
(#3559)
The spec does not mention traps here, but this is like a JS VM trapping on
OOM - a runtime limitation is reached.
As these are not specced traps, I did not add them to effects.h. Note how
as a result the optimizer happily optimizes into a nop an unused allocation of an
array of size unsigned(-1), which is the behavior we want.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
This matches #3747 which makes us not log out reference values, instead
we print just their types.
This also prints a type for non-reference things, replacing a previous
exception, which affects things like SIMD and BigInts, but those trap
anyhow at the JS boundary I believe (or did that change for SIMD?).
Anyhow, printing the type won't give a false "ok" when comparing wasm2js
output to the interpreter, assuming the interpreter prints out a value and
not just a type (which is the case). We could try to do better, but this
code is on the JS side, where we don't have the type - just a string
representation of it, which we'd need to parse etc.
|
| |
|
|
| |
Also removes experimental SIMD instructions that were not included in the final
spec proposal.
|
| |
|
|
| |
This is needed to make sure globals are printed before element segments,
where `global.get` can appear both as offset and an expression.
|
| |
|
|
| |
Instead of specifying the features in the wasm, enable them using
flags. This allows the testcase to be in text.
|
| |
|
|
|
|
|
| |
We've been keeping old syntax in the text format parser although they've
been removed from the parser and hardly any test case relies on them.
This PR will remove old syntax support for tables and element segments
and simplify the corresponding parser functions. A few test files were
affected by this that are updated.
|
| |
|
|
|
|
| |
This allows changing a global's value on the commandline in an easy way.
In some toolchains this is useful as the build can contain a global that
indicates something like a logging level, which this can customize.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This code used to remove functions it no longer thinks are needed. That is,
if it adds a legalized version of an import, it would remove the illegal
one which is no longer needed. To avoid removing an illegal import that
is still used it checked for ref.func appearances.
But this was bad in two ways:
We need to legalize the ref.funcs too. We can't call an illegal import
in any way, not using a direct call, indirect call, or call by reference of
a ref.func.
It's silly to remove unneeded functions here. We have a pass for that.
This removes the removal of functions, and adds proper updating of
ref.calls, which means to call the stub function that looks like the
original import, but that calls the legalized one and connects things
up properly, exactly the same way as other calls.
Also remove code that checked if we were in the stub/thunk and to
not replace the call there. That code is not needed: no one will ever
call the illegal import, so we do not need to be careful about
preserving such calls.
|
| |
|
|
|
|
|
|
| |
The passive keyword has been removed from spec's text format, and now
any data segment that doesn't have an offset is considered as passive.
This PR remove that from both parser and the Print pass, plus all tests
that used that syntax.
Fixes #2339
|
| |
|
|
|
|
|
|
|
|
|
|
| |
Log out an i64 as two i32s. That keeps the output consistent regardless of
whether we legalize.
Do not print a reference result. The printed value cannot be compared, as
funcref(10) (where 10 is the index of the function) is not guaranteed to be
the same after opts.
Trap when trying to call an export with a nondefaultable type (instead of
asserting when trying to create zeros for it).
|
| |
|
|
|
|
|
|
|
|
|
| |
In this case, there is a natural place to fix things up right after
removing a parameter (which is where a local gets added). Doing it
there avoids doing work on all functions unnecessarily.
Note that we could do something even simpler here than calling
the generic code: the parameter was not used, so the new local
is not used, and we could just change the type of the local or not
add it at all. Those would be slightly more code though, and add
complexity to the parameter removal method itself.
|
| |
|
|
|
|
|
|
|
| |
This was noticed by samparker on LLVM:
https://reviews.llvm.org/D99171
This is apparently a pattern LLVM emits, and doing it there helps by 1-2%
on the real-world Bullet Physics codebase. Seems worthwhile doing here
as well.
|
| |
|
|
|
|
|
| |
This is a partial revert of #3669, which removed the old implementation of
Type::getLeastUpperBound that did not correctly handle recursive types. The new
implementation in this PR uses a TypeBuilder to construct LUBs and for recursive
types, it returns a temporary HeapType that has not yet been fully constructed
to break what would otherwise be infinite recursions.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Several old passes like DeadArgumentElimination and DuplicateFunctionElimination
need to look at all ref.funcs, and they scanned functions for that, but that is not
enough as such an instruction might appear in a global initializer. To fix this, add a
walkModuleCode method.
walkModuleCode is useful when doing the pattern of creating a function-parallel
pass to scan functions quickly, but we also want to do the same scanning of code
at the module level. This allows doing so in a single line.
(It is also possible to just do walk() on the entire module, which will find all code,
but that is not function-parallel. Perhaps we should have a walkParallel() option
to simplify this further in a followup, and that would call walkModuleCode afterwards
etc.)
Also add some missing validation and comments in the validator about issues that
I noticed in relation to the new testcases here.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This implements emscripten-core/emscripten#13744
Inlining functions with a single use allows us to remove the function afterward.
That looks highly beneficial, shrinking every single benchmark in emscripten's
benchmark suite, by an average of 2% on the macrobenchmarks and 3.5% on
all of them. Speed also improves, although mostly on the microbenchmarks so
that might be less realistic.
There may be a slight downside to startup time due to emitting larger functions,
but given the baseline compilers in VMs these days it seems worth it, as the
delay would be just to get to the upper tier. On the benchmark suite the risk
seems low.
See more details in the PR above.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
#3719 optimized the case where the kind is what we want, like br_on_func of
a function. This handles the opposite case, where we know the kind is wrong, and
so the break is not taken.
This seems equally useful for "polymorphic" code that does a bunch of checks
and routes to different code for each case, as after inlining or other optimizations
we may see which paths are taken and which are not.
Also refactor the checking code to a shared location as RefIs/As will also want to
use it.
|
| |
|
| |
That pass adds lots of new locals, and we need to handle non-nullable ones.
|
| |
|
|
|
|
| |
This PR adds support for `ref.null t` as a valid element segment
item. The abbreviated format of `(elem ... func $f $g...)` is kept in
both printing and binary emitting if all items are `ref.func`s. Public
APIs aren't updated in this PR.
|
| |
|
|
| |
The type may prove the value is not null, and may also show it to be
of the type we are casting to. In that case, we can simplify things.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
After this PR we still do not support non-nullable locals. But we no longer
turn all types into nullable upon load. In particular, we support non-nullable
types on function parameters and struct fields, etc. This should be enough to
experiment with optimizations in both binaryen and in VMs regarding non-
nullability (since we expect that optimizing VMs can do well inside functions
anyhow; it's non-nullability across calls and from data that the VM can't be
expected to think about).
Let is handled as before, by lowering it into gets and sets. In addition, we
turn non-nullable locals into nullable ones, and add a ref.as_non_null on
all their gets (to keep the type identical there). This is used not just for
loading code with a let but also is needed after inlining.
Most of the code changes here are removing FIXMEs for allowing
non-nullable types. But there is also code to handle the issues mentioned
above.
Most of the test updates are removing extra nulls that we added before
when we turned all types nullable. A few tests had actual issues, though,
and also some new tests are added to cover the code changes here.
|
| |
|
|
|
| |
We have the if's type, and when replacing it with a select, can use that
type. This could be more efficient. It also avoids a current crash
after the removal of LUBs, but it's worth doing regardless of that.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
I'm not entirely sure how LUB removal made this noticeable, as it seems
to be a pre-existing bug. However, somehow before #3669 it was not
noticable - perhaps the finalize code worked around it.
The bug is that RemoveUnusedBrs was moving code around and
finalizing the parent before the child. The correct pattern is always to
work from the children outwards, as otherwise the parent is trying to
finalize itself based on non-finalized children.
The fix is to just not finalize in the stealSlice method. The caller can
do it after finishing any other work it has. As part of this refactoring,
move stealSlice into the single pass that uses it; aside from that being
more orderly, this method is really not a general-purpose tool, it is
quite specific to what RemoveUnusedBrs does, and it might easily
be used incorrectly elsewhere.
|
| |
|
| |
We were missing type names and the features section.
|
| |
|
|
| |
Also add more spec tests, including one that verifies we validate
rtt.sub and on a global location as fixed by #3694
|
