| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| ... | |
| | |
|
| |
|
|
|
|
|
| |
Since we optimize assuming a closed world, optimizations can change the types
and structure of GC data even in externally-visible ways. Because differences
are expected, the fuzzer already did not compare reference-typed values from
before and after optimizations when running with nominal typing. Update it to
not compare these values under any type system.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
(some.operation
(ref.cast .. (local.get $ref))
(local.get $ref)
)
=>
(some.operation
(local.tee $temp
(ref.cast .. (local.get $ref))
)
(local.get $temp)
)
This can help cases where we cast for some reason but happen to not use the
cast value in all places. This occurs in j2wasm in itable calls sometimes: The
this pointer is is refined, but the itable may be done with an unrefined pointer,
which is less optimizable.
So far this is just inside basic blocks, but that is enough for the cast of itable
calls and other common patterns I see.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
Fixes a longstanding problem with isorecursive canonicalization that only showed
up in MacOS and occasionally Windows builds. The problem was that
`RecGroupEquator` was not quite correct in the presence of self-references in
rec groups. Specifically, `RecGroupEquator` did not differentiate between
instances of the same type appearing across two rec groups where the type was a
self-reference in one group but not in the other.
The reason this only showed up occasionally on some platforms was that this bug
could only cause incorrect behavior if two groups that would incorrectly be
compared as equal were hashed into the same bucket of a hash map. Apparently the
hash map used on Linux never hashes the two problematic groups into the same
bucket.
|
| |
|
|
|
|
|
|
|
| |
(#5273)
When `-Wheader-hygiene` is enabled, C compiler will warn when using
namespace directive in global context in header file.
When `-Wimplicit-const-int-float-conversion` is enabled C compiler will
warn on implicit integer to double conversions that change values.
|
| |
|
|
| |
The previous error message was ambiguous and could easily be interpreted to mean
the opposite of what it meant.
|
| |
|
|
|
| |
(#5266)
This reverts commit 570007dbecf86db5ddba8d303896d841fc2b2d27.
|
| |
|
|
|
| |
This reverts commit b2054b72b7daa89b7ad161c0693befad06a20c90.
It looks like the necessary V8 change has not rolled out everywhere yet.
|
| |
|
|
| |
If the target is a bottom type then it is a heap type but it is not a signature
type, and we should treat it as unreachable (and not crash).
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
#5253 handled the case of just one possible global. It is also possible we have
multiple globals but just one value. This handles that case. (It slightly overlaps
with other passes, but as this pass actually identifies the creations of the objects
in globals, it has a guarantee of success that the others don't, and it is very easy
to just do given all the work done to handle the case of 2 values).
Also fix a minor bug in #5253 - we need to trap if the old reference were null.
That is, we know the reference must point to the only object ever created of
that type, but that is only if it is not null; if it's null we need to trap.
|
| |
|
|
| |
This is more modern and (IMHO) easier to read than that old C typedef
syntax.
|
| |
|
|
|
| |
Including support for parsing field indices. Although only numeric field indices
are supported at the moment, set up the code to make it straightforward to
implement type-dependent symbolic field names in the future.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Expand GlobalStructInference to operate on cases with a single possible global, and
not just 2 or more. Even the case of a single global is useful, it turns out, as we can
alter the reference in places like this:
(struct.get $type 0
(..ref..)
)
No matter what ref is, if there is a single global it must refer to, we can switch to
this:
(struct.get $type 0
(global.get $global)
)
That can unlock further opts later. Note that we can do this even if we don't know
what the value actually is - we may not know what the struct.get returns, but we
do know what it reads from.
|
| |
|
|
| |
They were optional for a while to allow users to gracefully transition to using
them, but now make them mandatory to match the upstream WasmGC spec.
|
| |
|
|
| |
Fixes #5250
|
| |
|
|
|
|
|
|
|
|
| |
Pretty simple logic bug, but it ended up causing us to not optimize sometimes.
Sadly the original tests happened to not have anything that depended on the
index in isolation.
Fix + add comprehensive tests for using that index properly. Also test the
call.without.effects intrinsic, which is orthoginal to this, but also worth testing
as it is a big use case here.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
| |
The offset and size were previously being sign extended from 32 to 64 bits,
which meant that negative sizes could make the bounds check pass and cause an
exception to be thrown by an overly large allocation. Switch to using uint64_t
from the start rather than mixing sizes and signs, and update the tests to
reproduce the error more robustly in the absence of the fix.
Also fix a bug in RemoveUnusedModuleElements triggered by the new test.
Fixes #5249.
|
| |
|
|
| |
All that code did was filter contents by the type of the RefCast. We do that for all
expressions now, so it was redundant.
|
| |
|
|
|
|
|
|
|
|
| |
We don't actually have the distributive property since our
PossibleContents representation is an approximation, and the fuzzer found
a case where that is noticeable. See more details in the new comment +
testcase.
I measured speed and memory usage and this actually causes almost no
noticeable change.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
First, we forgot to note the type annotation on `ArrayNewSeg` instructions, so
in small modules where these are the only annotated instructions, the type
section would be incomplete.
Second, in the interpreter we were reserving space for the array before checking
that the segment access was valid. This could cause huge allocations that threw
bad_alloc exceptions before the interpreter could get around to trapping. Fix
the problem by reserving the array after validating the arguements.
Fixes #5236.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Monomorphization finds cases where we send more refined types to a function
than it declares. In such cases we can copy the function and refine the parameters:
// B is a subtype of A
foo(new B());
function foo(x : A) { ..}
=>
foo_B(new B()); // call redirected to refined copy
function foo(x : A) { ..} // unchanged
function foo_B(x : B) { ..} // refined copy
This increases code size so it may not be worth it in all cases. This initial PR is
hopefully enough to start experimenting with this on performance, and so it does
not enable the pass by default.
This adds two variations of monomorphization, one that always does it, and the
default which is "careful": it sees whether monomorphizing lets the refined function
actually be better than the original (say, by removing a cast). If there is no
improvement then we do not make any changes. This saves a significant amount
of code size - on j2wasm the careful version increases by 13% instead of 20% -
but it does run more slowly obviously.
|
| |
|
| |
Per the wasm spec, memory.grow instructions should return -1 when there is a failure to allocate enough memory. This PR adds support for returning this error code.
|
| |
|
|
|
|
|
|
|
|
|
| |
OptimizeInstructions in rare cases can add unreachability. We propagate it out at
the end all at once. The fuzzer was smart enough to find a very special combination
of code + passes that can hit an issue, see the testcase.
As mentioned in the TODO, we should perhaps avoid adding unreachability in
OptimizeInstructions at all. If this happens again that might be worth the effort. But
also checking the type of the child as in this PR doesn't add much complexity in the
code.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Update MemoryPacking for array.new_data
The MemoryPacking pass looks at all instructions that reference memory segments
to determine how they can be optimized. #5214 introduced a new instruction that
references memory segments, array.new_data, but did not update MemoryPacking
accordingly. This omission meant that MemoryPacking could produce invalid or
misoptimized modules in the presence of array.new_data.
Fix the problem by making MemoryPacking aware of array.new_data. Consider
array.new_data when determining whether a segment is used and update
array.new_data to reflect the new, optimized segment numberings afterward. To
keep things simple, do not try to split any segment that is referred to by
a array.new_data instruction.
* fix
* Add test explanations
* Fix possible-contents.h for `array.new_{data,elem}`
This code was not properly updated in #5214, so GUFA would incorrectly optimize
out `array.new_data` and `array.new_elem` instructions. Fix the problem by
making these instructions data flow roots.
* fix
* move tests
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Update MemoryPacking for array.new_data
The MemoryPacking pass looks at all instructions that reference memory segments
to determine how they can be optimized. #5214 introduced a new instruction that
references memory segments, array.new_data, but did not update MemoryPacking
accordingly. This omission meant that MemoryPacking could produce invalid or
misoptimized modules in the presence of array.new_data.
Fix the problem by making MemoryPacking aware of array.new_data. Consider
array.new_data when determining whether a segment is used and update
array.new_data to reflect the new, optimized segment numberings afterward. To
keep things simple, do not try to split any segment that is referred to by
a array.new_data instruction.
* fix
* Add test explanations
|
| |
|
|
|
|
|
|
|
|
| |
Instead of automatically determining which exports will be async they will
be explicitly set by the user. We'll rely on the runtime trapping if they
are incorrectly set.
Two new arguments that behave similar to asyncify-imports:
- jspi-imports
- jspi-exports
|
| | |
|
| |
|
|
|
|
|
|
|
| |
In order to test them, fix the binary and text parsers to accept passive data
segments even if a module has no memory. In addition to parsing and emitting the
new instructions, also implement their validation and interpretation. Test the
interpretation directly with wasm-shell tests adapted from the upstream spec
tests. Running the upstream spec tests directly would require fixing too many
bugs in the legacy text parser, so it will have to wait for the new text parser
to be ready.
|
| |
|
| |
Adds support for the Asyncify pass to use Multi-Memories. This is specified by passing flag --asyncify-in-secondary-memory. Another flag, --asyncify-secondary-memory-size, is used to specify the initial and max size of the secondary memory.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Similar to #5194 but for RedundantSetElimination. This has similar benefits in terms
of using a more refined local in hopes of avoiding casts in followup opts, but unlike
SimplifyLocals this will operate across basic blocks.
To do this, we need to track not just local.set but also local.get in that pass. Then
in each basic block we can track the equivalent locals and pick from them.
I see a few dozen casts removed in the J2Wasm binary. Often stuff like this happens:
y = cast(x);
if (..) {
foo(x); // this could use y
}
|
| |
|
| |
We did not preserve the ordering of the fixed-size storage there.
|
| |
|
|
|
|
|
| |
These operations emit a completely different type than their input, so they must be
marked as roots, and not as things that flow values through them (because then
we filter everything out as the types are not compatible).
Fixes #5219
|
| |
|
| |
See: https://reviews.llvm.org/D125728
|
| |
|
|
|
|
|
|
|
|
|
|
| |
The binary parser was eagerly getting the name of memories to set the `memory`
field of data segments, but that meant that when the memory names were updated
later while parsing the names section, the data segment memory fields would
become out of date. Update the issue by deferring setting the `memory` fields
like we do for other parts of IR that reference memories.
Also fix a segfault in the validator that was triggered by the reproducer for
this bug before the bug was fixed.
Fixes #5204.
|
| |
|
|
|
|
|
|
|
|
|
| |
This can help in rare cases in MVP wasm, say for the return value of a block. But for
wasm GC it is very important due to casts.
Similar logic was added as part of #5194 for SimplifyLocals. It should probably have
been in a separate PR then. This does the right thing for RedundantSetElimination,
as a separate PR. Full tests will appear in that later PR (it is not really possible to test
the GC side yet - we need the logic in the later PR that actually switches to a more
refined local index when available).
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Adds C APIs to inspect compound struct, array and signature heap types:
Obtain field types, field packed types and field mutabilities of struct types:
BinaryenStructTypeGetNumFields (to iterate)
BinaryenStructTypeGetFieldType
BinaryenStructTypeGetFieldPackedType
BinaryenStructTypeIsFieldMutable
Obtain element type, element packed type and element mutability of array types:
BinaryenArrayTypeGetElementType
BinaryenArrayTypeGetElementPackedType
BinaryenArrayTypeIsElementMutable
Obtain parameter and result types of signature types:
BinaryenSignatureTypeGetParams
BinaryenSignatureTypeGetResults
|
| |
|
|
|
|
|
| |
We just checked if the new type we prefer (when switching a local to a more
refined one in #5194) is different than the old type. But that check at the end
must check it is a subtype as well.
Diff without whitespace is smaller.
|
| |
|
|
|
|
|
|
|
| |
This sorts globals by their usage (and respecting dependencies). If the module
has very many globals then using smaller LEBs can matter.
If there are fewer than 128 globals then we cannot reduce size, and the pass
exits early (so this pass will not slow down MVP builds, which usually have just
1 global, the stack pointer). But with wasm GC it is common to use globals for
vtables etc., and often there is a very large number of them.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
possible (#5194)
(local.set $refined (cast (local.get $plain)))
..
.. (local.get $plain) .. ;; we can change this to read from $refined
By using the more refined type we may be able to eliminate casts later.
To do this, look at the fallthrough value (so we can look through a cast or a block
value - this is the reason for the small wasm2js improvements in tests), and also
extend the code that picks which local index to read to look at types (previously
we just ignored any pairs of locals with different types).
|
| |
|
|
|
|
|
|
|
|
| |
E.g.
Atomic operation (atomics are disabled)
=>
Atomic operations require threads [--enable-threads]
|
| |
|
|
|
|
|
|
|
|
| |
Adds a multi-memories lowering pass that will create a single combined memory from the memories added to the module. This pass assumes that each memory is configured the same (type, shared).
This pass also:
- replaces existing memory.size instructions with a custom function that returns the size of each memory as if they existed independently
- replaces existing memory.grow instructions with a custom function, using global offsets to track the page size of each memory so data doesn't overlap in the singled combined memory
- adjusts the offsets of active data segments
- adjusts the offsets of Loads/Stores
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously the pass only pushed past an if or a br_if. This does the same but into an
if arm. On Wasm GC for example this can perform allocation sinking:
function foo() {
x = new A();
if (..) {
use(x);
}
}
=>
function foo() {
if (..) {
x = new A(); // this moved
use(x);
}
}
The allocation won't happen if we never enter the if. This helps wasm MVP too,
and in fact some existing tests benefit.
|
| |
|
|
|
|
|
|
|
| |
If a heap type only ever appears as the result of a read, we must include it in
the analysis in ModuleUtils, even though it isn't written in the binary format.
Otherwise analyses using ModuleUtils can error on not finding all types in the
list of types.
Fixes #5180
|
| | |
|
| |
|
|
|
|
|
| |
The fallthrough there is trickier because the value is evaluated before the condition.
Unlike other fallthroughs, the value is not last, so we need to check if the condition
(which is after it) interferes with it.
|
| |
|
| |
See #5188
|
| | |
|
| |
|
|
|
| |
This makes the logic symmetric and easier to read.
Measuring speed, this seems identical to before, so that concern seems fine.
|
