| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When we do a local.set of a value into a local then we have both a subtyping constraint - for
the value to be valid to put in that local - and also a flow of a value, which can then reach
more places. Such flow then interacts with casts in Unsubtyping, since it needs to know
what can flow where in order to know how casts force us to keep subtyping relations.
That regressed in the not-actually-NFC #6323 in which I added the innocuous lines
to add subtyping constraints in ref.eq. It seems fine to require that the arms of a
RefEq must be of type eqref, but Unsubtyping then assuming those arms flowed into
a location of type eqref... which means casts might force us to not optimize some
things.
To fix this, differentiate the rare case of non-flowing subtyping constraints, which is
basically only RefEq. There are perhaps a few more cases (like i31 operations) but they
do not matter in practice for Unsubtyping anyhow; I suggest we land this first to undo
the regression and then at our leisure investigate the other instructions.
|
| |
|
|
| |
Previously we lowered this to `getCodePointAt`, which has different semantics
around surrogate pairs.
|
| |
|
|
|
|
|
|
| |
Catch and report all kinds of WTF-8 encoding errors in the source strings,
including invalid leading bytes, invalid trailing bytes, unexpected ends of
strings, and invalid surrogate sequences. Insert replacement characters into the
output as necessary. Add a TODO about minimizing size by escaping only those
code points mandated to be escaped by the JSON spec. Generally improve
readability of the code.
|
| |
|
|
|
| |
A constant is either fixed up immediately, or does not need a call. This makes us
slightly faster in the fuzzer, but does not change behavior as before those calls all
ended up doing nothing (as the numbers were not nans).
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We already have passes to legalize i64 imports and exports, which the fuzzer will
run so that we can run wasm files in JS VMs. SIMD and multivalue also pose a
problem as they trap on the boundary. In principle we could legalize them as well,
but that is substantial effort, so instead just prune them: given a wasm module,
remove any imports or exports that use SIMD or multivalue (or anything else that
is not legal for JS).
Running this in the fuzzer will allow us to not skip running v8 on any testcase we
enable SIMD and multivalue for.
(Multivalue is allowed in newer VMs, so that part of this PR could be removed
eventually.)
Also remove the limitation on running v8 with multimemory (v8 now supports
that).
|
| | |
|
| |
|
|
|
|
|
|
| |
This replaces horrible hacks to find which nulls need to switch (from none to
noext) with general code using SubtypingDiscoverer. That helper is aware of
where each expression is written, so we can find those nulls trivially.
This is NFC on existing usage but should fix any remaining bugs with null
constants.
|
| |
|
|
| |
Also add an end-to-end test using node to verify we can parse the escaped
content properly using TextDecoder+JSON.parse.
|
| | |
|
| |
|
| |
StringAs's output must be non-nullable, so add a cast.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
The input module might use an array of 16-bit elements type that is somewhere in a
giant rec group, but that is not valid for imported strings: that array type is now on an
import and must match the expected ABI, which is to be in its own personal rec group.
The old array16 type remains in the module after this transformation, but all uses of it
are replaced with uses of the new array16 type.
Also move makeImports to after updateTypes: there are no types to update in the new
imports. That does not matter but it can make debugging less pleasant, so improve it.
|
| |
|
|
|
| |
Replacing the string heap type with extern is dangerous as they do not share top/bottom
types. In practice this works out almost everywhere except for a few ifs, which we can fix
up as a hack for now.
|
| |
|
|
| |
We want to actually remove all stringref appearances, in both public and
private types.
|
| |
|
| |
Arrays have immutable length, so we can optimize them like immutable fields.
|
| |
|
|
|
|
|
|
|
| |
Get as many of the lit tests as possible to parse with the new parser, mostly by
moving declared module items to be after imports. Also fix a bug in the new
parser's pop validation to allow supertypes of the expected type.
The two big issues that still prevent some lit tests from working correctly
under the new parser are missing support for symbolic field names and missing
support for source map annotations.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
SimplifyGlobals already does this, so this is a subset of that pass, and does not
add anything new. It is useful for testing, however.
In particular it allows testing that we propagate subsequent globals in a single
pass, that is if one global reads from another and becomes constant, then it
can be propagated as well. SimplifyGlobals runs multiple passes so this always
worked, but with this pass we can test that we do it efficiently in one pass.
This will also be useful for comparing stringref to imported strings, as it
allows gathered strings to be propagated to other globals (possible with
stringref, but not imported strings) but not anywhere else (which might have
downsides as it could lead to more allocations).
Also add an additional test for simplify-globals that we do not get confused by
an unoptimizable global.get in the middle (see last part).
|
| |
|
| |
All those in the list from #6271 (comment)
|
| |
|
|
|
| |
globals (#6285)
Before we propagated to the top level, but not to anything interior.
|
| |
|
|
|
|
|
|
| |
The new parser enforces the rule that imports must come before declarations
(except for type declarations). The old parser does not enforce this rule, so
many of our tests did not follow it. Fix them to follow that rule and fix other
invalid syntax. Also add missing finalization of Load expressions in
wasm-builder.h that was causing a test to fail under the new parser and guard
against an error case in wasm-ir-builder.cpp that used to cause a segfault.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Update identifiers used in tests to use a format supported by the new text
parser, i.e. either the standard format with its limited set of allowed
characters or the non-standard `$"..."` format. Notably, any name containing
square or curly braces now uses the string format.
Input automatically updated with this script:
https://gist.github.com/tlively/4e22311736661849e641d02e521a0748
The printer is updated to properly escape names in more places as well. The
logic for escaping names is moved to a common location so that the type
printing logic in wasm-type.cpp can use it as well.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This adds `--experimental-new-eh` option to `wasm-opt`. The difference
between this and `--translate-to-new-eh` is, `--translate-to-new-eh`
just runs `TranslateToNewEH` pass, while `--experimental-new-eh`
attaches `TranslateToNewEH` pass at the end of the whole optimization
pipeline. So if no other passes or optimization options (`-On`) are
specified, it is equivalent to `--translate-to-new-eh`. If other
optimization passes are specified, it runs them and at the end run the
translator to ensure the new EH instructions are emitted. The reason we
are doing this this way is that the optimization pipeline as a whole
does not support the new EH instruction yet, but we would like to
provide an option to emit a reasonably OK code with the new EH
instructions.
This also means when the optimization level > 3, it will also run
the StackIR + local2stack optimization after the translation.
Not sure how to test the output of this option, given that there is not
much point in testing the default optimization passes, and it is also
not clear how to print the stack IR if the stack ir generation and
optimization runs as a part of the pipeline and not the explicit command
line options.
This is created in favor of #6267, which added the option to
`optimization-options.h`. It had a problem of running the translator
multiple times when `-On` was given multiple times in the command line,
which I learned was rather a common usage. This adds the option directly
to `wasm-opt.cpp`, which avoids the problem. With this, it is still
possible to create and optimize Stack IR unnecessarily, but that feels a
better alternative.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This extends StringGathering by replacing the gathered string globals to imported
globals. It adds a custom section with the strings that the imports are expected to
provide. It also replaces the string type with extern.
This is a complete lowering of strings, except for string operations that are a TODO.
After running this, no strings remain in the wasm, and the outside JS is expected
to provide the proper imports, which it can do by processing the JSON of the
strings in the custom section "string.consts", which looks like
["foo", "bar", ..]
That is, an array of strings, which are imported as
(import "string.const" "0" (global $string.const_foo (ref extern))) ;; foo
(import "string.const" "1" (global $string.const_bar (ref extern))) ;; bar
|
| |
|
| |
Followup to #6243 which handled empty ones.
|
| |
|
| |
We only noted the type but not the literal value.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This adds `STACKIR-OPT` filecheck lines to `translate-to-new-eh.wast`
to see if StackIR's `local2stack` optimization successfully removes some
of unnecessary `local.set`/`local.get`s.
While supporting the whole Binayren optimization pipeline for the new EH
instructions is not the goal for the very near-term future, StackIR's
`local2stack` optimization can help with a very common pattern generated
by this translator, which is:
```wast
(try $l
(do ... )
(catch_all
(call $destructor)
(rethrow $l)
)
)
```
is translated to
```wast
(block $outer
(local.set $exn ;; can be optimized away
(block $catch_all (result exnref)
(try_table (catch_all_ref $catch_all)
...
)
(br $outer)
)
)
(call $destructor)
(throw_ref
(local.get $exn) ;; can be optimized away
)
)
```
Here we don't really need `local.set $exn` and `local.get $exn`, and
these can be optimized away using StackIR's local2stack. After
optimizing them away in Stack IR, the code can be like
```wast
block $outer
block $catch_all (result exnref)
try_table (catch_all_ref $catch_all)
...
end
br $outer
end
call $destructor
throw_ref
end
```
This optimization alone reduces the code size increased caused by
translating significantly. For Adobe Photoshop, the code size increase
goes down from 4.2% to 2.8%, and for Binaryen, it goes down from 3.8% to
2.0%.
|
| |
|
|
|
|
|
|
|
|
| |
This reverts commit 9090ce56fcc67e15005aeedc59c6bc6773220f11.
This has the effect of once more propagating string constants from
globals to other places (and from non-globals too), which is useful
for various optimizations even if it isn't useful in the final output.
To fix the final output problem, #6257 added a pass that is run at the
end to collect string.const to globals, which allows us to once more
propagate strings in the optimizer, now without a downside.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This pass finds all string.const and creates globals for them. After this transform, no
string.const appears anywhere but in a global, and each string appears in one global
which is then global.get-ed everywhere.
This avoids overhead in VMs where executing a string.const is an allocation, and is
also a good step towards imported strings. For that, this pass will be extended from
gathering to a full lowering pass, which will first gather into globals as this pass does,
and then turn each of those globals with a string.const into an imported externref.
(For that reason this pass is in a file called StringLowering, as the two passes will
share much of their code, and the larger pass should decide the name I think.)
This pass runs in -O2 and above. Repeated executions have no downside (see
details in code).
|
