| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| ... | |
| |
|
|
|
|
|
|
| |
Now that all known issues with that pass are fixed, enable it by
default. This adds it in a place that seems to make sense on j2wasm,
but in general multiple cycles of optimization will be needed.
This adds a test showing that we run this pass and that it helps
ConstantFieldPropagation by running before it.
|
| |
|
|
|
| |
The BrOn logic there is incremental in optimizing and updating types, and so
we cannot assume that at every point in the middle the types are fully
updated.
|
| |
|
| |
Fixes #4308.
|
| |
|
|
|
| |
Generate both nullable and non-nullable references to basic HeapTypes and
introduce `i31` and `data` HeapTypes. Generate subtypes rather than exact types
for all concrete-typed children.
|
| |
|
|
|
|
|
|
|
|
|
| |
We only need to use locals if there are effects we can't remove, or if they
interact with other children. Improve the comment to explain what the
ChildLocalizer is working towards: a state where all the children of the
expression can be reordered or removed freely (local.gets have that
property, as do other things if they have no relevant effects).
Aside from avoiding wasteful locals, this is necessary for running
GlobalTypeOptimization on j2wasm: That code will do a global.get
of an rtt, and those cannot be placed in locals.
|
| |
|
|
| |
In preparation for using it from a separate file specifically for generating
random HeapTypes that has no need to depend on all of fuzzing.h.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
Allocation and cast instructions without explicit RTTs should use the canonical
RTTs for the given types. Furthermore, the RTTs for nominal types should reflect
the static type hierarchy. Previously, however, we implemented allocations and
casts without RTTs using an alternative system that only used static types
rather than RTT values. This alternative system would work fine in a world
without first-class RTTs, but it did not properly allow mixing instructions that
use RTTs and instructions that do not use RTTs as intended by the M4 GC spec.
This PR fixes the issue by using canonical RTTs where appropriate and cleans up
the relevant casting code using std::variant.
|
| |
|
|
|
|
|
|
|
| |
This is a minor refactoring in DAE to have a helper class that does the
incremental LUB calculation. The class is also used in LocalSubtyping,
where it has the effect of making the work incremental which it was not
before (that would have no observable consequence, but it should make
us faster in the common case where we fail to find a new LUB).
This will allow further optimization in a central place later.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Similar to what we do with structs, if a global is immutable then we know it
cannot interact with calls.
This changes the JS API for getSideEffects(). That was actually broken,
as passing in the optional module param would just pass it along to the
compiled C code, so it was coerced to 0 or 1, and not a pointer to a module.
To fix that, this now does module.ptr to actually get the pointer, and this is
now actually tested as without a module we cannot compute the effects of a
global. This PR also makes the module param mandatory in the JS API,
as again, without a module we can't compute global effects. (The module
param has already been mandatory in the C++ API for some time.)
|
| |
|
|
| |
This helps prevent bugs where we assume that the GCData has either a HeapType or
Rtt without checking. Indeed, one such bug is found and fixed.
|
| |
|
|
|
|
|
| |
When the allocation we optimize away flows through a loop, then just like
with a block we must change the type to be nullable, since we are replacing
the allocation with a null.
Fixes #4287
|
| | |
|
| |
|
|
|
| |
We have separate logic for printing their headers and bodies, and they were not in sync.
Specifically, we would not emit drops in the body of a block, which is not valid, and would
fail roundtripping on text.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If we write an immutable global to a field, and that is the only thing
we ever write, then we can replace reads of the field with a get of
the global. To do that, this tracks immutable globals written to
fields and not just constant values.
Normally this is not needed, as if the global is immutable then we
propagate its constant value to everywhere anyhow. However, for
references this is useful: If we have a global immutable vtable,
for example, then we cannot replace a get of it with a constant.
So this PR helps with immutable reference types in globals, allowing
us to propagate global.gets to them to more places, which then
can allow optimizations there.
This + later opts removes 25% of array.gets from j2wasm. I believe
almost all of those are itable calls, so this means those are getting
devirtualized now. I see something like a 5% speedup due to that.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Canonicalize:
(signed)x > -1 ==> x >= 0
(signed)x <= -1 ==> x < 0
(signed)x < 1 ==> x <= 0
(signed)x >= 1 ==> x > 0
(unsigned)x < 1 ==> x == 0
(unsigned)x >= 1 ==> x != 0
This should help #4265, and in general 0 is usually a more
common constant, and reasonable to canonicalize to.
|
| |
|
|
|
| |
Without this, we'd just return the old type for the tuple, which meant
its fields referred to unrewritten types, and possible validation errors
if the types changed.
|
| |
|
|
|
|
| |
Having a monolithic header file containing all the implementation meant there
was no good way to split up the code or introduce new files. The new
implementation file and source directory will make it much easier to add new
fuzzing functionality in new files.
|
| |
|
| |
Saves a little code size and might prevent some bugs.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
(#4263)
If struct.new operands have side effects, and we are removing the operand
as the field is removed, we must keep the side effects. To handle that, store
all the operands in locals and read from the locals, and then removing a
local.get is always safe to do, and nothing has been reordered:
(struct.new
(A)
(side effect) ;; this field will be removed
(B)
)
=>
(local.set $a (A))
(local.set $t (side effect))
(local.set $b (B))
(struct.new
(local.get $a)
(local.get $b)
)
Later passes can remove unneeded local operations etc.
This is necessary before enabling this pass, as this corner case occurs on
j2wasm.
|
| |
|
|
|
|
| |
Do so by applying --debug to extraFlags right at the start. That global
is used everywhere already. In particular, this PR removes manually adding
-g in the first diff chunk here, and you can see extraFlags appears there
already on the previous line.
|
| |
|
|
| |
This adds support for `try`-`delegate` to `CFGWalker`. This also adds a
single test for `catch`-less `try`.
|
| |
|
|
|
|
|
| |
The current code the innermost (`i`th) case specially first and handles
`i-1`th `try` in each loop iteration. This puts the `i`th case in the
loop and each iteration handles `i`th `try`, which is simpler. Then we
don't need to check `throwingInstsStack.empty()` in the beginning
because the `for` loop wouldn't be entered if it's empty anyway.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Add struct.get tracking, and if a field is never read from, simply remove
it.
This will error if a field is written using struct.new with a value with side
effects. It is not clear we can handle that, as if the struct.new is in a
global then we can't save the other values to locals etc. to reorder
things. We could perhaps use other globals for it (ugh) but at least for
now, that corner case does not happen on any code I can see.
This allows a quite large code size reduction on j2wasm output (20%). The
reason is that many vtable fields are not actually read, and so removing
them and the ref.func they hold allows us to get rid of those functions,
and code that they reach.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We already detected code that looks like
if (foo == 0) {
foo = 1;
}
That "read only to write" pattern occurs also in functions, like this:
function bar() {
if (foo == 0) return;
foo = 1;
}
This PR detects that pattern. It moves code around to share almost
all the logic with the previous pattern (the git diff is not that useful
there, sadly, but looking at them side by side that should be
obvious).
This helps in j2cl on some common clinits, where the clinit function
ends up empty, which is exactly this pattern.
|
| |
|
| |
Fuzzing followup to #4244.
|
| |
|
|
|
| |
Code in the If condition can be moved out to before the if.
Existing test updates are 99% whitespace.
|
| |
|
| |
This sets the C++ standard variable in the build to C++17, and makes use of std::optional (a C++17 library feature) in one place, to test that it's working.
|
| | |
|
| |
|
|
|
|
|
|
|
| |
Just as the --nominal flag forces all types to be parsed as nominal, the
--structural flag forces all types to be parsed as equirecursive. This is the
current default behavior, but a future PR will change the default to parse types
as either structural or nominal according to their syntax or encoding. This new
flag will then be necessary to get the current behavior.
Also take this opportunity to deduplicate more flags in the help tests.
|
| |
|
|
| |
Very simple with the work so far, just add StructGet/ArrayGet code to check
if the field is immutable, and allow the get to go through in that case.
|
| |
|
|
|
|
| |
This adds support for tag-using instructions (`throw` and `catch`) to
wasm-metadce. We had to use a hacky workaround in
emscripten-core/emscripten#15266 because of the lack of this support;
after this lands we can remove it.
|
| |
|
|
|
|
|
|
| |
Switch from "extends" to M4 nominal syntax
Change all test inputs from using the old (extends $super) syntax to using the
new *_subtype syntax for their inputs and also update the printer to emit the
new syntax. Add a new test explicitly testing the old notation to make sure it
keeps working until we remove support for it.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This optimizes this type of pattern:
(local.set $x (struct.new X Y Z))
(struct.set (local.get $x) X')
=>
(local.set $x (struct.new X' Y Z))
Note how the struct.set is removed, and X' moves to where X was.
This removes almost 90% (!) of the struct.sets in j2wasm output, which reduces
total code size by 2.5%. However, I see no speedup with this - I guess that either
this is not on the hot path, or V8 optimizes it well already, or the CPU is making
stores "free" anyhow...
|
| | |
|
| |
|
|
|
|
|
|
|
| |
Not sure why the current code tries to add the name even when it is
null, but it causes `dump()` to behave strangely and pollute stdout when
it tries to print `root.str`.
Also this changes code printing `Name.str` to printing just `Name`; when
`Name.str` is null, it prints `(null Name)` instead of polluting stdout,
and it is the recommended way of printing `Name` anyway.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
Precompute will run the interpreter on struct.new etc. repeatedly,
as it keeps doing so while it propagates constant values around (if one
of the operands to the struct.new becomes constant, that could have
a noticeable effect). But creating new GC data means we lose track of
their identity, and so ref.eq would not work, and we disabled basically
all struct operations. This implements identity tracking so we can start
to optimize there, which is a step towards using it for immutable field
propagation.
To track identity, always store the data representing each struct.new
in the source using the same GCData structure. That keeps identity
consistent no matter how many times we execute.
|
| |
|
|
|
|
|
|
|
|
|
| |
Side effects in the first element are always ok there, as they are
not moved across anything else: they happen before their parent
both before and after the opt.
The pass just left ternary as a TODO, so do at least one part of
that now (we can do the rest as well, with some care).
This is fairly useful on array.set which has 3 operands, and the
first often has interesting things in it.
|
| |
|
|
| |
This makes Binaryen match LLVM on a real-world case, which is probably
the safest heuristic to use.
|
| | |
|
| |
|
|
|
|
| |
This is the easy part of using immutability more: Just note immutable
fields as such when we read from them, and then a write to a struct
does not interfere with such reads. That is, only a read from a mutable
field can notice the effect of a write.
|
| |
|
|
|
|
|
|
|
|
|
| |
Add an assert on not emitting a null name (which would cause
a crash a few lines down on trying to read its bytes). I hit that
when writing a buggy pass that updated field names.
Also fix the case of a type not having a name but some of its
fields having names. We can't test that atm since our text
format requires types to have names anyhow, so this is a
fix for a possible future where we do allow parsing non-named
types.
|
| |
|
|
|
|
|
| |
Div/rem by a constant can be optimized by VMs, so it is usually
closer to the speed of a mul.
Div on 64-bit (either with or without a constant) can be slower
than 32-bit, so bump that up by one as well.
|
| |
|
|
|
| |
`BinaryenTableSizeSetTable` was being declared in the header correctly, but defined
as `BinaryenTableSetSizeTable`. Add test for `BinaryenTableSizeGetTable` and
`BinaryenTableSizeSetTable`.
|
| |
|
|
|
| |
We moved call_ref out of there, but it was still checking for the possible
presence of call_refs (using the feature), which means that even if we had
no valid tables to optimize on, we'd scan the whole module.
|
| |
|
|
|
|
|
|
|
| |
This method is in parallel to runOnFunction above it. It sets the runner
and then does the walk, like that method.
Also set runner to nullptr by default. I noticed ubsan was warning on
things here, which this should avoid, but otherwise I'm not aware of an
actual bug, so this should be NFC. But it does provide a safer API
that should avoid future bugs.
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
| |
Implement parsing the new {func,struct,array}_subtype format for nominal types.
For now, the new format is parsed the same way the old-style (extends X) format
is parsed, i.e. in --nominal mode types are parsed as nominal but otherwise they
are parsed as equirecursive. Intentionally do not parse the new types
unconditionally as nominal for now to allow frontends to update their nominal
text format while continuing to use the workflow of running wasm-opt without
--nominal to lower nominal types to structural types.
|
| |
|
|
|
|
|
|
| |
See #4220 - this lets us handle the common case for now of simply having
an identical heap type to the table when the signature is identical.
With this PR, #4207's optimization of call_ref + table.get into
call_indirect now leads to a binary that works in V8 in nominal mode.
|
