| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
|
|
| |
This change loops through memories to validate each meets wasm spec. Also factors data segment validation out from memory validation, as it makes more sense for data segments to stand alone like the other module-level elements.
|
|
|
|
|
| |
Just like `extern` is no longer a subtype of `any` in the new GC type system,
`func` is no longer a subtype of `any`, either. Make that change in our type
system implementation and update tests and fuzzers accordingly.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This fixes what looks like it might be a regression in #4943. It's not actually
an issue since it just affects wat files, but it did uncover an existing
inefficiency. The situation is this:
(block
..
(br $somewhere)
(nop)
)
Removing such a nop is always helpful, as the pass might see that that
br goes to where control flow is going anyhow, and the nop would
confuse it. We used to remove such nops only when the block had a name,
which is why wat testcases looks optimal, but we were actually doing the
less-efficient thing on real-world code. It was a minor inefficiency, though, as
the nop is quickly removed by later passes anyhow. Still, the fix is trivial (to
always remove such nops, regardless of a name on the block or not).
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously the wat parser would turn this input:
(block
(nop)
)
into something like this:
(block $block17
(nop)
)
It just added a name all the time, in case the block is referred to by an index
later even though it doesn't have a name.
This PR makes us rountrip more precisely by not adding such names: if there
was no name before, and there is no break by index, then do not add a name.
In addition, this will be useful for non-nullable locals since whether a block has
a name or not matters there. Like #4912, this makes us more regular in our
usage of block names.
|
|
|
|
|
|
|
| |
Some fuzzer initial contents contain non-nullable externrefs that cause the
fuzzer to try to materialize non-nullable externref values. Perviously the
fuzzer did not support this and crashed with an assertion failure. Fix the
assertion failure by instead returning a null cast to non-null, which will trap
at runtime but at least produce a valid module.
|
|
|
|
|
|
|
| |
Such globals can be written to from the outside, so we cannot infer anything about
their contents.
Fixes #4932
|
|
|
|
|
|
| |
Resolving a couple of issues from the multi-memories PR landing:
Use memName as parameter label instead of name #4916
Add helper func for case of a single memory to binaryen-c #4917
|
|
|
|
|
|
|
|
| |
In LLVM output and probably others, the initial table contents are never
changed. We may append later, but we don't trample the initial table
entries. As a result, with this new flag we can turn indirect calls on those
offsets into direct ones:
--directize-initial-tables-immutable
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
casts (#4720)
i32 -> f64 -> i32 rountripping optimizations:
```rust
i32(f64(i32(x))) -> x // i32.trunc(_sat)_f64_s(f64.convert_i32_s(x)) -> x
u32(f64(u32(x))) -> x // i32.trunc(_sat)_f64_u(f64.convert_i32_u(x)) -> x
// note assymetric signed / unsigned or unsigned / signed cases can't be simplified in similar way
```
and rounding after integer to float casts:
```rust
ceil(float(int(x))) -> float(int(x))
floor(float(int(x))) -> float(int(x))
trunc(float(int(x))) -> float(int(x))
nearest(float(int(x))) -> float(int(x))
```
where `float = f32 | f64`, `int = i32 | i64 | u32 | u64`
|
|
|
|
| |
Due to missing test coverage, we missed in #4811 that some memory operations
needed to get make64() called on them.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
A rather tricky corner case: we normally look at fallthrough values for copies of
fields, so when we try to refine a field, we ignore stuff like this:
a.x = b.x;
That copies the same field on the same type to itself, so refining is not limited by
it. But if we have something else in the middle, and that thing cannot change
type, then it is a problem, like this:
(struct.set
(..ref..)
(local.tee $temp
(struct.get)))
tee has the type of the local, which does not change in this pass. So we can't
look at just the fallthrough here and skip the tee: after refining the field, the
tee's old type might not fit in the field's new type.
We could perhaps add casts to fix things up, but those may have too big a
cost. For now, just ignore the fallthrough.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
DAE will normally not remove an unreachable parameter, because it checks for
effects there. But in TrapsNeverHappen mode, we assume that an unreachable
is an effect we can remove, so we are willing to remove it:
(func $foo (param $unused i32)
;; never use $unused
)
(func $bar
(call $foo
(unreachable)))
;;=> dae+tnh
(func $foo
)
(func $bar
(call $foo))
But that transformation is invalid: the call's type was unreachable before but
no longer is. What went wrong here is that, yes, it is valid to remove an
unreachable, but we may need to update types while doing so, which we
were not doing.
This wasn't noticed before due to a combination of unfortunate factors:
The main reason is that this only happens in TrapsNeverHappens mode. We
don't fuzz that, because it's difficult: that mode can assume a trap never happens,
so a trap is undefined behavior really. On real-world code this is great, but in the
fuzzer it means that the output can seem to change after optimizations.
The validator happened to be missing an error for a call that has type unreachable
but shouldn't: Validator: Validate unreachable calls more carefully #4909 . Without
that, we'd only get an error if the bad type influenced a subsequent pass in a confusing
way - which is possible, but difficult to achieve (what ended up happening in practice is
that SignatureRefining on J2Wasm relied on the unreachable and refined a type too much).
Even with that fix, for the problem to be detected we'd need for the validation error to
happen in the final output, after running all the passes. In practice, though, that's not
likely, since other passes tend to remove unreachables etc. Pass-debug mode is very
useful for finding stuff like this, as it validates after every individual pass. Sadly it turns
out that global validation was off there: Validator: Validate globally by default #4906
(so it was catching the 99% of validation errors that are local, but this particular error
was in the remaining 1%...).
As a fix, simply ignore this case. It's not really worth the effort to optimize it, since DCE
will just remove unreachables like that anyhow. So if we run again after a DCE we'd get
a chance to optimize.
This updates some existing tests to avoid (unreachable). That was used as an example
of something with effects, but after this change it is treated more carefully. Replace those
things with something else that has effects (a call).
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Normally the validator will find stale types properly, by just running refinalize and seeing
if the type has changed (if so, then some code forgot to refinalize). However, refinalize
is a local operation, so it does not apply to calls: a call's proper type is determined by
the global information of the function we are calling. As a result, we would not notice
errors like this:
(call $foo) ;; type: unreachable
Refinalizing that would not change the type from unreachable to the proper type, since
that is global information.
To validate this properly, validate that a call whose type is unreachable actually has
an unreachable child. That rules out an invalid unreachable type here, which leaves
concrete types, that we already have proper global validation for. The code here is
generalized to handle non-call things as well, but it only helps expressions requiring
global validation, so it likely only helps global.get and a few others.
|
|
|
|
|
|
|
|
|
|
| |
We already did this if the block was a child of a control flow structure, which is
the common case (see the new added comment around that code, which clarifies
why). This does the same for all other blocks. This is simple to do and a minor
optimization, but the main benefit from this is just to make our handling of blocks
uniform: after this, we never emit a block with no name. This will make 1a non-
nullable locals easier to handle (since they will be able to assume that property;
and not emitting such blocks avoids some work to handle non-nullable locals
in them).
|
|
|
|
|
|
|
| |
The GC proposal has split `any` and `extern` back into two separate types, so
reintroduce `HeapType::ext` to represent `extern`. Before it was originally
removed in #4633, externref was a subtype of anyref, but now it is not. Now that
we have separate heaptype type hierarchies, make `HeapType::getLeastUpperBound`
fallible as well.
|
|
|
|
|
|
|
| |
This PR removes the single memory restriction in IR, adding support for a single module to reference multiple memories. To support this change, a new memory name field was added to 13 memory instructions in order to identify the memory for the instruction.
It is a goal of this PR to maintain backwards compatibility with existing text and binary wasm modules, so memory indexes remain optional for memory instructions. Similarly, the JS API makes assumptions about which memory is intended when only one memory is present in the module. Another goal of this PR is that existing tests behavior be unaffected. That said, tests must now explicitly define a memory before invoking memory instructions or exporting a memory, and memory names are now printed for each memory instruction in the text format.
There remain quite a few places where a hardcoded reference to the first memory persist (memory flattening, for example, will return early if more than one memory is present in the module). Many of these call-sites, particularly within passes, will require us to rethink how the optimization works in a multi-memories world. Other call-sites may necessitate more invasive code restructuring to fully convert away from relying on a globally available, single memory pointer.
|
|
|
|
|
|
| |
I'm not sure why this defaulted to non-global. Perhaps because of limitations
in the asm.js days. A better default is to validate globally, and this also applies
in pass-debug mode (since that just uses the default there), so this will catch
more problems there.
|
|
|
|
|
|
|
| |
The validation logic to check for stale types (code where we forgot to run
refinalize) had a workaround for a control flow issue. That workaround meant
we didn't catch errors where a type was concrete but it should be unreachable.
This PR makes that workaround only apply for control flow structures, so we can
catch more errors.
|
|
|
|
|
|
|
| |
Reverts #4889
The spec is unclear on this, and that PR moved us to do what V8 does. But
it sounds like we should clarify the spec to do things the other way, so this
goes back to that.
|
|
|
|
|
|
|
|
|
|
| |
call.without.effects has a specific form, where the last parameter is a
function reference, and that function reference must have the right type
for the other parameters if called with them:
(call $call.without.effects
(..i32..)
(..f64..)
(..function reference, which takes params i32 and f64..)
|
|
|
|
|
|
| |
This allows emscripten to move these helper functions from JS library
imports to native wasm exports.
See https://github.com/emscripten-core/emscripten/issues/7273
|
|
|
|
|
|
| |
In BINARYEN_PASS_DEBUG=2 we save the module before each pass, and if
validation fails afterwards, we print the module before. This PR does the same for
function-parallel passes - in that case, we can actually show the specific function
that broke validation, as opposed to the whole module.
|
|
|
|
| |
`pop`s type should be a supertype, not a subtype, of the tag's type
within `catch`.
|
|
|
| |
Like the 8-bit array variants, it takes 3 parameters.
|
|
|
|
|
|
|
| |
For now this index is always 0, but we must emit it.
Also clean up the wat test a little - we don't have validation yet, but we should
not validate without a memory in that file.
|
|
|
|
|
|
| |
This starts to matter with strings, it turns out. This change should make us
runnable in v8.
Spec: https://github.com/WebAssembly/gc/blob/main/proposals/gc/MVP.md#instructions-1
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
call.without.effects will turn into a normal call of the last parameter later,
(call $call.without.effects
A
B
(ref.func $foo)
)
;; => intrinsic lowering
(call $foo
A
B
)
SignaturePruning needs to be aware of that: we can't remove a parameter from $foo without
also updating relevant calls to $call.without.effects. Rather than handle that, just skip such
cases, and leave them to be optimized after intrinsics are lowered away.
|
|
|
|
|
|
|
| |
A function literal (ref.func) should never reach a struct or array get, but
if there is a cast then it can look like they might arrive. We filter in ref.cast
which avoids that (since casting a function to a data type will trap), but
there is also br_on_cast which is not yet optimized. This PR adds code
to avoid an assert in readFromData in that case.
|
|
|
|
|
|
| |
eqz(eqz(i32(x))) -> i32(x) != 0
eqz(eqz(i64(x))) -> i64(x) != 0
Only when shrinkLevel == 0 (prefer speed over binary size).
|
|
|
|
| |
This is no longer needed by emscripten as of:
https://github.com/emscripten-core/emscripten/pull/16529
|
|
|
|
|
|
|
|
| |
stripping data segments. (#4876)
This avoid a fatal crash in `--post-emscripten` where it tries to remove
data that is no longer part of the file.
This fixes bug introduced by #4871 that causes emscripten tests to fail.
|
|
|
|
|
|
|
| |
RTTs were removed from the GC spec and if they are added back in in the future,
they will be heap types rather than value types as in our implementation.
Updating our implementation to have RTTs be heap types would have been more work
than deleting them for questionable benefit since we don't know how long it will
be before they are specced again.
|
|
|
|
| |
Rather than doing it as a side effect of dumping the metadata in
wasm-emscripten-finalize.
|
| |
|
|
|
|
|
|
|
|
|
| |
`--debug`. NFC (#4874)
For example I found it useful to able to do something like this:
```
$ BINARYEN_DEBUG=post-emscripten ./test/runner sometest
```
|
|
|
| |
Introduces the necessary APIs to use the type builder from C. Enables construction of compound heap types (arrays, structs and signatures) that may be recursive, including assigning concrete names to the built types and, in case of structs, their fields.
|
| |
|
|
|
|
|
|
|
|
| |
We already remove `__start_em_asm` and `__stop_em_asm`. This change
is needed since I want to start exporting `__start_em_js` and
`__stop_em_js` from emscripten without causing regressions.
As a followup I'm planning on moving all of the em_js and em_asm
stripping code it PostEmscripten.cpp.
|
|
|
|
|
| |
It has been removed from the typed function references proposal, so we no longer
need to support it. Maintaining the test for `let` was difficult because
Binaryen could not emit either text or binary that actually used it.
|
|
|
|
|
|
|
|
|
|
|
|
| |
+ Move these rules to separate function;
+ Refactor them to use matches;
+ Add comments;
+ Handle rotational shifts as well;
+ Handle overflows for `<<`, `>>`, `>>>` shifts;
+ Add mixed rotate rules:
```rust
rotl(rotr(x, C1), C2) => rotr(x, C1 - C2)
rotr(rotl(x, C1), C2) => rotl(x, C1 - C2)
```
|
|
|
| |
Avoids a "may fall through" warning.
|
| |
|
|
|
|
|
|
| |
We already require non-null literals to have non-null types, but with this
change we can enforce that constraint by construction. Also remove the default
behavior of creating a function reference literal with heap type `func`, since
there is always a more specific function type to use.
|
|
|
| |
This can give us some chance to catch bugs like #4839 in the fuzzer.
|
|
|
|
|
|
|
|
| |
Like RemoveUnusedModuleElements, places that build graphs of function
reachability must special-case the call-without-effects intrinsic. Without that,
it looks like a call to an import. Normally a call to an import is fine - it makes us
be super-pessimistic, as we think things escape all the way out - but in GC
for now we are assuming a closed world, and so we end up broken. To fix that,
properly handle the intrinsic case.
|
|
|
|
|
|
| |
It was wasted work to see a drop and then check if we can replace it with
a drop of its child, which is identical to the original state. This didn't cause
any harm (we'd not reduce code size, and stop eventually) but it did slow us
down.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Refactor everywhere from:
```c++
for (size_t i = 0; i < indent; i++) {
o << ' ';
}
```
to:
```c++
o << std::string(indent, ' ');
```
### Motivation
It is much simpler and should produce smaller code.See godbolt:
https://godbolt.org/z/KMYMdn7z5
|