| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| ... | |
| |
|
|
| |
Turns out we had a testcase for this already, but were doing the
wrong thing on it.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
In `ReFinalize`'s branch handling, `updateBreakValueType` is supposed to
be executed only when the branch itself is not replaced with its
argument (because it is guaranteed not to be taken).
Also this moves `visitBrOnExn` from `RuntimeExpressionRunner` to its
base class `ExpressionRunner`, because it does not depend on anything on
the runtime instance to work. This is effectively NFC for now because
`visitTry` is still only implemented only in `RuntimeExpressionRunner`
because it relies on multivalue handling of it, and without it we cannot
create a valid exception `Literal`.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously we were able to omit the new syntax `do` when `try` body is
empty. This makes `do` clause mandatory, so when a `try` body is empty,
the folded text format will be
```
(try
(do)
(catch
...
)
```
Suggested in
https://github.com/WebAssembly/exception-handling/issues/52#issuecomment-626696720.
|
| |
|
|
|
|
|
| |
This adds support for `throw`, `rethrow`, and `br_on_exn` in
MergeBlocks. While unrelated instructions within blocks can be hoisted
as in other instructions, `br_on_exn` requires a special handling in
`ProblemFinder`, because unlike `br_if`, its `exnref` argument itself
cannot be moved out of `br_on_exn`.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In WebAssembly/exception-handling#52, We decided to put `try` bodies in
a `do` clause to be more consistent with `catch`.
- Before
```wast
(try
...
(catch
...
)
)
```
- After
```wast
(try
(do
...
)
(catch
...
)
)
```
Another upside of this change is when there are multiple instructions
within a `try` body, we no longer need to wrap them in a `block`.
|
| |
|
|
| |
This adds missing handlings for `throw` and `rethrow` in DCE. They
should set `reachable` variable to `false`, like other branches.
|
| |
|
|
|
|
|
| |
This allows emscripten to statically set the initial value of the
stack pointer.
Should allow use to avoid doing it dynamically at startup:
https://github.com/emscripten-core/emscripten/pull/11031
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We had some ad-hoc tuning of which nodes to emit more
frequently in the fuzzer, but it wasn't very good. Things like
loads and stores for example were far too rare. Also it wasn't
easy to adjust the frequencies.
This adds a simple way to adjust them, by passing a size_t
which is the "weight" of that node. Then it just makes that
number of copies of it, making it more likely to be picked.
Example output comparison:
node before after
================================
binary 281 365
block 898 649
break 278 144
call 182 290
call_indirect 9 42
const 808 854
drop 43 92
global.get 440 398
global.set 223 171
if 335 254
load 22 84
local.get 429 301
local.set 434 211
loop 176 99
nop 117 54
return 264 197
select 8 33
store 1 39
unary 405 304
unreachable 1 2
Lots of noise here obviously, but there are large increases
for loads and stores compared to before.
Also add a testcase of random data of the typical size the
fuzzer runs, and print metrics on it. This might help us get
a feel for how future tuning changes affect frequencies.
|
| |
|
| |
Since the --roundtrip pass is more general than --fuzz-binary anyways. Also reimplements `ModuleUtils::clearModule` to use the module destructor and placement new to ensure that no members are missed.
|
| |
|
|
| |
type. fixes #2807 (#2808)
|
| |
|
|
|
|
|
| |
Refactors most of the precompute pass's expression runner into its
base class so it can also be used via the C and JS APIs. Also adds
the option to populate the runner with known constant local and global
values upfront, and remembers assigned intermediate values as well
as traversing into functions if requested.
|
| |
|
|
|
|
|
|
| |
Emit tuple.make, tuple.extract, and multivalue control flow, and tuple locals
and globals when multivalue is enabled. Also slightly refactors the top-level
`makeConcrete` function to be more selective about what it tries to
make based on the requested type to reduce the number of trivial nodes
created because the requested type is incompatible with the requested
node.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The main benefit here is comparing VMs, instead of just comparing
each VM to itself after opts. Comparing VMs is a little tricky since there
is room for nondeterminism with how results are printed and other
annoying things, which is why that didn't work well earlier.
With this PR I can run 10's of thousands of iterations without finding
any issues between v8 and the binaryen interpreter. That's after
fixing the various issues over the last few days as found by this:
#2760 #2757 #2750 #2752
Aside from that main benefit I ended up adding more improvements
to make it practical to do all that testing:
Randomize global fuzz settings like whether we allow NaNs and
out-of-bounds memory accesses. (This was necessary here since
we have to disable cross-VM comparisons if NaNs are enabled.)
Better logging of statistics like how many times each handler
was run.
Remove redundant FuzzExecImmediately handler (looks like
after past refactorings it was no longer adding any value).
Deterministic testcase handling: if you run e.g. fuzz_opt.py 42 it
will run one testcase and exactly the same one. If you run without
an argument it will run forever until it fails, and if it fails, it prints out
that ID so that you can easily reproduce it (I guess, on the same
binaryen + same python, not sure how python's deterministic
RNG changes between versions and builds).
Upgrade to Python 3.
|
| |
|
|
|
|
|
|
|
|
| |
Previously we tried to reuse `Const` node if a precomputed value is a
constant node. But now we have two more kinds of constant node
(`RefNull` and `RefFunc`), so we shouldn't reuse them interchangeably,
meaning we shouldn't try to reuse a `Const` node when the value at hand
is a `RefNull`. This correctly checks the type of node and tries to
reuse only if the types of nodes match.
Fixes #2759.
|
| |
|
|
|
|
| |
When it is certain that the try body does not throw, we can replace the
try-catch with the try body. But in this case we have to notify the type
updater that the catch body is removed, so that all parents' type should
be updated properly.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
I am working to bring up the fuzzer on comparisons between VMs.
Comparing between the binaryen interpreter and v8, it found some
atomics issues:
Atomic operations, including loads and stores, must be aligned
or they trap.
AtomicRMW did the wrong thing with the operands.
AtomicCmpxchg must wrap the input to the proper size (if we
only load 1 byte, only look at 1 byte of the expected value too).
AtomicWait and AtomicNotify must take into account their
offsets. Also SIMDLoadExtend was missing that. This was
confusing in the code as two getFinalAddresses existed,
one that doesn't compute with an offset, and one that does.
I renamed the one without to getFinalAddressWithoutOffset
so it's explicit and we can easily see we only call that one on
an instruction without an offset (which is the case for
MemoryInit, MemoryCopy, and MemoryFill).
AtomicNotify must check its address to see if it should trap,
even though we don't actually have multiple threads running.
Atomic loads of fewer bytes than the type always do an
unsigned extension, not signed.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
We used std::cerr as a workaround for that this logging
interfered with spec testing. But it's easy enough to filter
out this stuff for the spec tests.
The benefit to using std::cout is that as you can see in
the test output here, this is relevant test output - it's not
a side channel for debugging. If the rest of the interpreter
output is in std::cout but only traps are in std::cerr then
they might end up out of order etc., so best to keep them
all together.
This will allow easier additions of tests for fuzz testcases
|
| |
|
|
| |
We shouldn't actually nop, we forgot that the value may have
side effects, so just drop it (opts will remove it later, if possible).
|
| |
|
|
|
| |
The fuzzer was previously unconditionally emitting one event parameter
more than it was supposed to, which meant multivalue events were
emitted when multivalue was not enabled.
|
| |
|
|
| |
Based on freedback in #2741 it looks like we can use the existing
`simplify-globals-optimizing` pass to trigger this cleanups we need.
|
| |
|
|
|
| |
Since the global is never read, we know that any write operation
will be unobservable.
|
| |
|
|
|
| |
Instead of adding globals for hardcoded basic types, traverse the
module to collect all call types that might need to be handled and
emit a global for each of them. Adapted from #2712.
|
| |
|
|
|
|
|
|
| |
`BinaryIndexes` was only used in two places (Print.cpp and
wasm-binary.h), so it didn't seem to be a great fit for
module-utils.h. This change moves it to wasm-binary.h and removes its
usage in Print.cpp. This means that function indexes are no longer
printed, but those were of limited utility and were the source of
annoying noise when updating tests, anyway.
|
| |
|
|
|
|
| |
Unless the multivalue feature is enabled. The validation for events
recently changed to disallow events returning multiple items unless
the multivalue feature is enabled, but the fuzzer was not updated
accordingly. This PR fixes the glitch.
|
| |
|
|
| |
Update it from wasm-emscripten-finalize when we append
to the table.
|
| |
|
|
|
|
|
|
|
| |
Previously the multivalue feature enabled tuples in control flow
positions, but tuples elsewhere did not require the multivalue
feature. However, allowing tuple operations and locals in MVP modules
means that all passes and tools need to support tuples, even if it
isn't a high priority for them to support multivalue. Allowing tuples
in MVP modules doesn't provide much value, so this changes disallows
them entirely unless multivalue is enabled.
|
| |
|
| |
Iterate over tuple locals and separately load or store each component.
|
| |
|
|
|
| |
RemoveUnusedBrs produces selects for some patterns, but selects of
multivalue types are not valid. This change checks that types are not
tuple types before producing selects.
|
| |
|
|
|
|
|
|
| |
Also makes it work with any other constant expression such as a
ref.func or ref.null instructions. This optimization may not be very
important, but it illustrates how simple it can be to update a pass to
handle tuples (and also I was already looking at it because of the
prior changes that had to be made to it).
|
| |
|
|
|
|
| |
This involves replacing `Literal::makeZero` with `Literal::makeZeroes`
and `Literal::makeSingleZero` and updating `isConstantExpression` to
handle constant tuples as well. Also makes `Literals` its own struct
and adds convenience methods on it.
|
| |
|
|
|
|
|
|
|
|
| |
Such a module can't have valid DIEs, since we have no way to
interpret them.
Also check if DWARF sections from LLVM have contents -
when they are empty the section may exist but have a null
for its data.
Fixes #2673
|
| |
|
|
|
|
|
|
|
|
| |
Normally, a wrapper has to track state separately to know when to
unwind/rewind and when to actually call import functions.
Exposing Asyncify state can help avoid this duplication and avoid
subtle bugs when internal and wrapper state get out of sync.
Since this is a tiny function and it's useful for any Asyncify
embedder, I've decided to expose it by default rather than hide behind an option.
|
| |
|
|
|
|
|
|
|
|
|
| |
This does two things:
- Treats the target branch of `br_on_exn` as unoptimizables, because it
is a conditional branch.
- Makes sure we don't move expressions that contain `exnref.pop`, which
should follow right after `catch`.
- Adds `containsChild` utility function, which can search all children,
optionally with limited depth. This was actually added to be used in
CodeFolding but ended up not being used, but wasn't removed in case
there will be uses later.
|
| |
|
|
| |
This adds support for UniqueNameMapper, and adds a test in Inlining
pass, which uses UniqueNameMapper.
|
| |
|
|
|
|
|
|
|
| |
Like `br_if`, `br_on_exn` is a conditional branch and across which code
can be pushed past when conditions are satisfied.
Also adds a few lines of comments and NFC changes in a couple places.
Changes in Vacuum are NFC because they were being handled in `default:`
in the same way anyway, but I added them to be more explicit and
consistent with existing code.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
If an invalid entry appears - either it began as such, or became
invalid after optimization - we should not emit (0, 0) which is
an end marker. Instead, emit an invalid entry marker, something
with (0, x) for x != 0.
As a bonus, if a test/passes case has "noprint" in the name,
don't print the wasm, which we do by default. In the testcase
here for example we just care about the dwarf, and the
printed module would be quite large.
Thank you to @paolosevMSFT for identifying and suggesting
the fix.
|
| |
|
|
| |
If try's body does not throw, the whole try-catch can be replaced with
the try body.
|
| |
|
|
|
|
|
|
|
|
|
| |
Anything that merges/swaps/etc. locals, or inlines, or merges functions,
must be disabled for now. However, that does still leave almost all
passes, so this should not affect output sizes much (and the full LLVM
optimizer can be run before too).
Over time we can resolve each of those FIXMEs.
The test output here shows how disabling those allows over twice as
much debug_line info to be preserved.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Now that we have subtypes, we cannot reuse any local that contains the
same expression, because that local's type can be a supertype. For
example:
```
(local $0 anyref)
(local $1 nullref)
...
(local.set $0 (ref.null))
(local.set $1 (ref.null)) ;; cannot be replaced with (local.get $0)
```
This extends `usables` map's key to contain both `HashedExpression` and
the local's type, so we can get the right usable local in presence of
subtypes.
|
| |
|
|
|
|
| |
- Adds support for `Try` in `optimizeBoolean` function
- Adds support for `Try` in `getFallThrough` function
- Adds approximate cost values for instructions in EH and reference
types proposals.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This adds EH support to `EffectAnalyzer`. Before `throw` and `rethrow`
conservatively set property. Now `EffectAnalyzer` has a new property
`throws` to represent an expression that can throw, and expression that
can throw sets `throws` correctly.
When EH is enabled, any calls can throw too, so we cannot reorder them
with another expression with any side effects, meaning all calls should
be treated in the same way as branches when evaluating `invalidate`.
This prevents many reorderings, so this patch sets `throws` for calls
only when the exception handling features is enabled. This is also why I
passed `--disable-exception-handling` to `wasm2js` tests. Most of code
changes outside of `EffectAnalyzer` class was made in order to pass
`FeatureSet` to it.
`throws` isn't always set whenever an expression contains a throwable
instruction. When an throwable instruction is within an inner try, it
will be caught by the corresponding inner catch, so it does not set
`throws`.
|
| |
|
|
|
|
|
|
|
| |
Each compilation unit's abbreviations must be terminated by
a zero, so that we use the right abbreviations. This adds that
support to the YAML layer, both adding the zeros and parsing
them to look in the right abbreviation section at the right time.
Also add two large testcases, zlib and cubescript, which
crash without this and the last PR.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This replaces imports like env.foo with a.foo, which can
save a bunch of bytes when there are many imported
functions.
Note that by changing all the import names to a it ends
up requiring a single merged import module.
Note also that when doing this we modify all the imports,
minifying their modules and names (since it makes no
sense to be careful about minifying only modules known
to us - env/wasi - if we are minifyin the names of all
modules).
This will require an emscripten PR to benefit from it.
|
| |
|
|
|
|
|
|
|
| |
The LLVM SData field is 64-bit (to support 64-bit
addresses I suppose) so when we assigned to it we
actually led it to emit an LEB for a signed 64-bit value
that is an unsigned 32-bit one. This worked in LLVM
(where I guess it forces the value to 32-bit anyhow?)
but failed in gimli (where I guess it doesn't?).
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
We ignored them, which is a bad default, as typically they imply
we can call anything in the table (and the table might change).
Instead, notice indirect calls during traversal, and force the user
to decide whether to ignore them or not.
This was only an issue in PostEmscripten because the other
user, Asyncify, already had indirect call analysis because it
needed it for other things.
Fixes a bug uncovered by #2619 and fixes the current binaryen
roll.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Add support for that section to the YAML layer, and add
code to update it.
The updating is slightly tricky - unlike .debug_ranges, the
size of entries is not fixed. So we can't just skip entries,
as the end marker is smaller than a normal entry. Instead,
replace now-invalid segments with (1, 1) which is of size
0 and so should be ignored by the debugger (we can't use
(0, 0) as that would be an end marker, and (-1, *) is
the special base marker).
In the future we probably do want to do this in a more
sophisticated manner, completely rewriting the indexes
into the section as well. For now though this should be
enough for when binaryen does not optimize (as we
don't move/reorder anything).
Note that this doesn't update the location description
(like where on the wasm expression stack the value is).
Again, that is correct for when binaryen doesn't
optimize, but for fully optimized builds we would need
to track things (which would be hard!).
Also clean up some code that uses "Extra" instead of
"Delimiter" that was missed before, and shorten some
unnecessarily long names.
|
| |
|
|
|
|
| |
Chrome is currently decoding the segment indices as signed numbers, so
some ranges of indices greater than 63 do not work. As a temporary
workaround, limit the number of segments produced by MemoryPacking to
63 when bulk-memory is enabled.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Pretty straightforward given all we have so far.
Note that fannkuch3_manyopts has an example of
a sequence of ranges of which some must be skipped
while others must not, showing we handle that by
skipping the bad ones and updating the remaining. That
is, if that we have a sequence of two (begin, end) spans
[(10, 20),
(30, 40)]
It's possible (10, 20) maps in the new binary to (110, 120)
while (30, 40) was eliminated by the optimizer and we have
nothing valid to map it to. In that case we emit
[(110, 120)]
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Just some trivial fixes:
* Properly reset prologue after each line (unlike others, this
flag should be reset immediately).
* Test for a function's end address first, as LLVM output appears to
use 1-past-the-end-of-the-function as a location in that function,
and not the next (note the first byte of the next function, which is
ambiguously identical to that value, is used at least in low_pc;
I'm not sure if it's used in debug lines too).
* Ignore the same address if LLVM emitted it more than once, which
it does sometimes.
|
| |
|
|
|
|
|
| |
We need to track end_sequence directly, and use either
end_sequence or copy (copy emits a line without marking
it as ending a sequence).
After this, fib2 debug line output looks perfect.
|
