| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| ... | |
| |
|
|
|
|
|
|
| |
`shouldBeRef` incorrectly assumed that all `throw_ref`s within a `catch`
body had been generated from `rethrow`s, which was not true, because
`throw_ref`s are also created when translating `try`-`delegate`s:
https://github.com/WebAssembly/binaryen/blob/219e668e87b012c0634043ed702534b8be31231f/src/passes/TranslateEH.cpp#L304
This fixes the assumption and changes `cast` to `dynCast`.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When translating a `try` expression, we may need an 'outer' block that
wraps the newly generated `try_table` so we can jump out of the
expression when an exception does not occur. (The condition we use is
when the `try` has any catches or if the `try` is a target of any
inner `try-delegate`s:
https://github.com/WebAssembly/binaryen/blob/219e668e87b012c0634043ed702534b8be31231f/src/passes/TranslateEH.cpp#L677)
In case the `try` has either of `catch` or `delegate`, when we have the
'outer' block, we add the newly created `try_table` in the 'outer' block
and replace the whole expression with the block:
https://github.com/WebAssembly/binaryen/blob/219e668e87b012c0634043ed702534b8be31231f/src/passes/TranslateEH.cpp#L670
https://github.com/WebAssembly/binaryen/blob/219e668e87b012c0634043ed702534b8be31231f/src/passes/TranslateEH.cpp#L332-L340
But in case of a catchless `try`, we forgot to do that:
https://github.com/WebAssembly/binaryen/blob/219e668e87b012c0634043ed702534b8be31231f/src/passes/TranslateEH.cpp#L388
So this PR fixes it.
|
| |
|
|
|
| |
All Struct/Array operations must ignore mutable fields in Precompute atm, which we
did, but StringNew has an array variant that does an effective ArrayGet operation,
which we didn't handle.
|
| |
|
|
|
|
|
|
|
| |
struct.new_with_default (#6523)
Before we preferred not to add default values, as that increases code size. But since
#6495 we turn more things into struct.new_with default, so it is important to handle
this. It seems likely that in most cases the code size downside of adding default
values is offset by avoiding a local.set later, so always do this (rather than add some
kind of heuristic).
|
| |
|
|
|
|
|
|
|
|
|
| |
hen the function return type is concrete, the translation result of a
`try`-`delegate` that targets the caller includes a `return` that
returns the whole function body:
https://github.com/WebAssembly/binaryen/blob/219e668e87b012c0634043ed702534b8be31231f/src/passes/TranslateEH.cpp#L751-L763
We should do that based on the function's return type, not the function
body's return type. The previous code didn't handle the case where the
function's return type is concrete but the function body's return type
is unreachable.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Converts the following:
(some.operation
(ref.cast .. (local.tee $ref ..))
(local.get $ref)
)
into:
(some.operation
(local.tee $temp
(ref.cast .. (local.tee $ref ..))
)
(local.get $temp)
)
|
| |
|
|
|
|
|
|
|
|
|
| |
The code had a different workaround, namely to add a block with an explicit
type to avoid changing the type at all (i.e. the block declared the original
type of the thing we replaced, not the refined type). But by adding a block we
can end up with an invalid pop, as the fuzzer found, see the EH testcase here.
To fix this, use the usual workaround of just running ReFinalize. That is simpler
and also improves the code while we do it. It does add more work, but this is
likely a very rare situation anyhow.
|
| |
|
|
|
|
|
|
|
|
| |
For types that do not have explicit names, we generate index-based names in the
printer. However, we did not previously ensure that the generated types were not
already used as explicit names, so it was possible to print the same name for
multiple types, which is not valid.
Fix the problem by skipping indices that are already used as type names.
Fixes #6492.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
A few tests were creating files with the `%s` prefix, which expands to the test
filename itself, so the test files were being created in the test directory and
were never cleaned up. Another test was creating files called `%a.wast` and
`%b.wast`, but `%a` and `%b` do not expand to anything, so this also polluted
the test directory and caused problems with running other tests and fuzzing.
Use the proper `%t` prefix to create all these files in a temporary location
instead.
Fixes #6429.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
struct.new with default values can be struct.new_default.
array.new with default values can be array.new_default.
array.new of size 1 should be array.new_fixed (saves the Const).
array.new_fixed with default values can be array.new_default.
array.new_fixed with equal but non-default values can be array.new
(basically use a Const to say how many copies we want, rather than copy).
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The latest idea for efficient string constants is to encode the constants in
the import names of their globals and implement fast paths in the engines for
materializing those constants at instantiation time without needing to parse
anything in JS. This strategy only works for valid strings (i.e. strings without
unpaired surrogates) because only valid strings can be used as import names in
the WebAssembly syntax.
Add a new configuration of the StringLowering pass that encodes valid string
contents in import names, falling back to the JSON custom section approach for
invalid strings.
To test this chang, update the printer to escape import and export names
properly and update the legacy parser to parse escapes in import and export
names properly. As a drive-by, remove the incorrect check in the parser that the
import module and base names are non-empty.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
OptimizeInstructions::areConsecutiveInputsEqual() (#6481)
"Generative" is what we call something like a struct.new that may be syntactically
identical to another struct.new, but each time a new value is generated. The same
is true for calls, which can do anything, including return a different value for
syntactically identical calls.
This was not a bug because the main user of isGenerative,
areConsecutiveInputsEqual(), was too weak to notice, that is, it gave up sooner,
for other reasons. This PR improves that function to do a much better check,
which makes the fix necessary to prevent regressions.
This is not terribly important for itself, but will help a later PR that will add code
that depends more heavily on areConsecutiveInputsEqual().
|
| |
|
|
|
|
|
| |
GUFA already truncated packed fields on write, which is enough for unsigned gets,
but for signed gets we also need to sign them on reads.
Similar to #6493 but for GUFA. Also found by #6486
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Heap2Local's (#6493)
CFP already had logic for truncating but not for sign-extending, which this
fixes.
Use the new helper function in Heap2Local as well. This changes the model
there from "truncate on set, sign-extend on get" to "truncate or sign-extend
on get". That is both simpler by reusing the same logic as CFP but also more
optimal: the idea to truncate on sets made sense since sets are rarer, but if
we must then sign-extend on gets then we can end up doing more work
overall (as the truncations on sets are not needed if all gets are signed).
Found by #6486
|
| |
|
|
|
|
| |
We previously printed the size of the tuple operand as the arity, but that
printed `1` when the operand is unreachable. We don't allow our text input to
use `1` as the arity, so don't print it, either. Instead, print the smallest
valid arity, `2`, in this case.
|
| |
|
|
|
|
| |
In #6480 I forgot that StructGet can be signed, which means we need to emit
a sign-extend.
Arrays already copied the field as part of Array2Struct.
|
| |
|
|
|
| |
Use the previous implementation when no return_call is in a try block. This
avoids moving code around (as a sibling of the caller body or the inlined body),
so that should allow more local optimizations after inlining.
|
| |
|
|
|
|
| |
Previously we did not optimize a struct or an array with a packed field. As a result a
single packed field in a struct prevented the entire struct from being localized,
which this fixes. This is also useful for arrays as packed arrays are common (e.g. for
string data).
|
| |
|
|
|
|
|
|
|
|
|
|
| |
To keep things simple, this adds a Array2Struct component to the pass. When we
find a non-escaping array, we run that to turn it into a struct, and then run the
existing Struct2Local to convert that to locals. This avoids refactoring Struct2Local
to handle both structs and arrays (with the downside of making the optimization of
arrays a little less efficient, but they are rarer, I suspect - that is certainly the case
in Java output I've seen).
The core EscapeAnalyzer logic is generalized to handle both arrays and structs,
but the changes there are thankfully quite minor.
|
| |
|
|
| |
#6457 added a test that exposed existing nondeterminism.
|
| |
|
|
| |
Treat them the same as returns and test that they can be folded out of try-catch
blocks because they do not have throws effects.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is a combined commit covering multiple PRs fixing the handling of return
calls in different areas. The PRs are all landed as a single commit to ensure
internal consistency and avoid problems with bisection.
Original PR descriptions follow:
* Fix inlining of `return_call*` (#6448)
Previously we transformed return calls in inlined function bodies into normal
calls followed by branches out to the caller code. Similarly, when inlining a
`return_call` callsite, we simply added a `return` after the body inlined at the
callsite. These transformations would have been correct if the semantics of
return calls were to call and then return, but they are not correct for the
actual semantics of returning and then calling.
The previous implementation is observably incorrect for return calls inside try
blocks, where the previous implementation would run the inlined body within the
try block, but the proper semantics would be to run the inlined body outside the
try block.
Fix the problem by transforming inlined return calls to branches followed by
calls rather than as calls followed by branches. For the case of inlined return
call callsites, insert branches out of the original body of the caller and
inline the body of the callee as a sibling of the original caller body. For the
other case of return calls appearing in inlined bodies, translate the return
calls to branches out to calls inserted as siblings of the original inlined
body.
In both cases, it would have been convenient to use multivalue block return to
send call parameters along the branches to the calls, but unfortunately in our
IR that would have required tuple-typed scratch locals to unpack the tuple of
operands at the call sites. It is simpler to just use locals to propagate the
operands in the first place.
* Fix interpretation of `return_call*` (#6451)
We previously interpreted return calls as calls followed by returns, but that is
not correct both because it grows the size of the execution stack and because it
runs the called functions in the wrong context, which can be observable in the
case of exception handling.
Update the interpreter to handle return calls correctly by adding a new
`RETURN_CALL_FLOW` that behaves like a return, but carries the arguments and
reference to the return-callee rather than normal return values.
`callFunctionInternal` is updated to intercept this flow and call return-called
functions in a loop until a function returns with some other kind of flow.
Pull in the upstream spec tests return_call.wast, return_call_indirect.wast, and
return_call_ref.wast with light editing so that we parse and validate them
successfully.
* Handle return calls in wasm-ctor-eval (#6464)
When an evaluated export ends in a return call, continue evaluating the
return-called function. This requires propagating the parameters, handling the
case that the return-called function might be an import, and fixing up local
indices in case the final function has different parameters than the original
function.
* Update effects.h to handle return calls correctly (#6470)
As far as their surrounding code is concerned return calls are no different from
normal returns. It's only from a caller's perspective that a function containing
a return call also has the effects of the return-callee. To model this more
precisely in EffectAnalyzer, stash the throw effect of return-callees on the
side and only merge it in at the end when analyzing the effects of a full
function body.
|
| |
|
|
| |
Helps emscripten-core/emscripten#17380 by logging all the reasons why we
instrument a function, and not just the first as we did before.
|
| |
|
|
|
|
| |
Separate out an EscapeAnalyzer class that does the escape analysis,
and a Struct2Local one that does the optimization.
Also make a few things const here to be safer.
|
| |
|
|
| |
Since there was no difference between the WITHOUT and DISCARD check prefixes, we
can just use WITHOUT for both and reduce the number of check lines by a third.
|
| |
|
|
|
| |
There is no need to test the asyncified module output - we have tons of tests for
that. What this test should be testing is the verbose output which states which
things it decided to instrument, so make it do that.
|
| | |
|
| |
|
|
|
| |
The new asyncify flag --pass-arg=asyncify-propagate-addlist changes the
behavior of --pass-arg=asyncify-addlist : with it, callers of functions in the
asyncify-addlist will be also instrumented.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
This change removes the "minimal" mode from `LegalizeJSInterface`
which was added in #1883.
The idea behind this change was to avoid legalizing most function except
those we know that JS will be calling. The idea was that for dynamic
linking we always want the non-legalized version to be shared between
wasm module. These days we solve this problem in a different way with
the `legalize-js-interface-export-originals` which exports the original
functions alongside the legalized ones. Emscripten then always
prefers the `$orig` functions when doing dynamic linking.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
WTF-16, i.e. arbitrary sequences of 16-bit values, is the encoding of Java and
JavaScript strings, and using the same encoding makes the interpretation of
string operations trivial, even when accounting for non-ascii characters.
Specifically, use little-endian WTF-16.
Re-encode string constants from WTF-8 to WTF-16 in the parsers, then back to
WTF-8 in the writers. Update the constructor for string `Literal`s to interpret
the string as WTF-16 and store a sequence of WTF-16 code units, i.e. 16-bit
integers. Update `Builder::makeConstantExpression` accordingly to convert from
the new `Literal` string representation back to a WTF-16 string.
Update the interpreter to remove the logic for detecting non-ascii characters
and bailing out. The naive implementations of all the string operations are
correct now that our string encoding matches the JS string encoding.
|
| |
|
|
| |
`string.encode_wtf16_array` operates on stringref, not on wtf16 string views, so
this conversion was causing validation errors when passed to V8 by the fuzzer.
|
| |
|
|
| |
Our validator apparently does not catch this type issue yet. For now just fix
the tests.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
The only StringEncode we support is the one that writes into an array, so it
has the same effects as ArrayCopy. Precompute needs to be made aware of
such side effects in a manual manner (as we already do for ArrayCopy etc.):
it simply tries to execute code in the interpreter, and if it succeeds it replaces;
it does not check for side effects (checking for side effects would prevent
optimizing cases where the side effects do not happen, as we check them
statically, e.g. dividing by a non-zero constant does not trap but a division
would be seen as having a potential trap effect).
I verified no other string operation is hit by this: all the others
emit or operate on immutable strings; it is just StringEncode that is basically
an Array operation that appears in the Strings proposal.)
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The pass does (among other things) this:
(if
condition
X
X
)
=>
(block
(drop
condition
)
X ;; deduplicated
)
After that the condition is now nested in a block, so we may need EH fixups
if it contains a pop.
|
| |
|
|
|
|
|
|
| |
This reverts commit 70ac213fce134840609190a5d3a18118a089ba8a.
Reverts #6412
On second thought we found a way to make fixing this less urgent, and the
code size downsides of this are worrying, so let's revert it.
|
| |
|
|
| |
Our UTF implementation is still not fully stable it seems as we have reports of
issues. Disable it for now.
|
| | |
|
| | |
|
| |
|
|
|
|
|
| |
Our interpreter implementations of `stringview_wtf16.length`,
`stringview_wtf16.get_codeunit`, and `string.encode_wtf16_array` are not
unicode-aware, so they were previously incorrect in the face of multi-byte code
units. As a fix, bail out of the interpretation if there is a non-ascii code
point that would make our naive implementation incorrect.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
effects (#6395)
Before this PR, when we saw a param was unused we sometimes could not remove it.
For example, if there was one call like this:
(call $target
(call $other)
)
That nested call has effects, so we can't just remove it from the outer call - we'd need to
move it first. That motion was hard to integrate which was why it was left out, but it
turns out that is sometimes very important. E.g. in Java it is common to have such calls
that send the this parameter as the result of another call; not being able to remove such
params meant we kept those nested calls alive, creating empty structs just to have
something to send there.
To fix this, this builds on top of #6394 which makes it easier to move all children out of
a parent, leaving only nested things that can be easily moved around and removed. In
more detail, DeadArgumentElimination/SignaturePruning track whether we run into effects that
prevent removing a field. If we do, then we queue an operation to move the children
out, which we do using a new utility ParamUtils::localizeCallsTo. The pass then does
another iteration after that operation.
Alternatively we could try to move things around immediately, but that is quite hard:
those passes already track a lot of state. It is simpler to do the fixup in an entirely
separate utility. That does come at the cost of the utility doing another pass on the
module and the pass itself running another iteration, but this situation is not the most
common.
|
| | |
|
| |
|
|
|
|
|
|
| |
We incorrectly overrode the string operations in the interpreter's subclasses. But
string operations can be implemented in the topmost class there (as they depend on
no module state), so just implement them there, once, in a proper way.
This fixes StringEq by removing its override, and moves the others to the right
place.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is NFC in the current users, but is necessary functionality for a later
PR.
ChildLocalizer moves children into locals as needed. It used to stop when it
saw the first unreachable. After this change we move such unreachable
children out of the parent as well, making this more uniform: all interacting
effects are moved out, and all that is left nested in the parent can be
moved around and removed as desired.
Also add a getReplacement helper that makes using this easier.
This cannot be tested comprehensively with the current user as that user
will not call this code path on an unreachable parent at all, so this just
adds what can be tested. The later PR will have tests for all corner cases.
|
| | |
|
| |
|
|
|
|
|
|
|
| |
When the bulk array ops had unreachable or null array types, they were replaced
with blocks, but not using the correct code that also prints all their children
as dropped followed by an unreachable. This meant that the text output in those
cases did not parse as a valid module.
Fix the bug. A follow-up PR will simplify the code to prevent similar bugs from
occurring in the future.
|
| |
|
|
|
| |
We previously required a memory to exist while parsing all `StringNew` and
`StringEncode` instructions, even though some variants of the instructions use
GC arrays instead. Require a memory only for those instructions that use one.
|
| |
|
|
|
|
|
|
|
|
| |
In some cases we don't print an Expression in full if it is unreachable, so
we print something instead as a placeholder. This happens in unreachable
code when the children don't provide enough info to print the parent (e.g.
a StructGet with an unreachable reference doesn't know what struct type
to use).
This PR prints out the name of the Expression type of such things, which
can help debugging sometimes.
|
| |
|
| |
Adds new visitBreakWithType and visitSwitchWithType functions to the IRBuilder API. These functions work around an assumption in IRBuilder that the module is being traversed in the fully nested format, i.e., that the destination scope of a break or switch has been visited before visiting the break or switch. Instead, the type of the destination scope is passed to IRBuilder.
|
