| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
|
|
|
| |
When a parameter and a member variable have the same name within a
constructor, to access (and change) the member variable, we need to
either use `this->` or change the name of the parameter. The current
code ended up changing the parameter and didn't affect the status of the
member variable, which remained empty.
|
| |
|
|
|
| |
Update the LUB calculation code to use std::optional rather than out params and
validate LUBs in the fuzzer to ensure that the change is NFC as intended. Also
add HeapType::getLeastUpperBound to the public API as a convenience.
|
| |
|
|
|
|
|
|
|
| |
When a wasm exception is thrown and uncaught in the interpreter, it
caused the whole interpreter to crash, rather than gracefully reporting
it. This fixes the problem, and also compares whether an uncaught
exception happened when comparing the results before and after
optimizations in `--fuzz-exec`. To do that, when `--fuzz-exec` is given,
we now compare results even when the function does not have return
values. Logs for some existing test have changed because of this.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
With nominal function types, this change makes it so that we preserve the
identity of the function type used with call_indirect instructions rather than
recreating a function heap type, which may or may not be the same as the
originally parsed heap type, from the function signature during module writing.
This will simplify the type system implementation by removing the need to store
a "canonical" nominal heap type for each unique signature. We previously
depended on those canonical types to avoid creating multiple duplicate function
types during module writing, but now we aren't creating any new function types
at all.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This adds `EHUtils::handleBlockNestedPops`, which can be called at the
end of passes that has a possibility to put `pop`s inside `block`s. This
method assumes there exists a `pop` in a first-descendant line, even
though it can be nested within a block. This allows a `pop` to be nested
within a `block` or a `try`, but not a `loop`, since that means the
`pop` can run multile times. In case of `if`, `pop` can exist only in
its condition; if a `pop` is in its true or false body, that's not in
the first-descendant line.
This can be useful when optimization passes create blocks to do
transformations. Wrapping expressions wiith a block does not change
semantics most of the time, but if pops happen to be inside a block
generated by those passes, they can result in invalid binaries.
To test this, this adds `passes/test_passes.cpp`, which is intended to
contain multiple test passes that test a single (or more) utility
functions separately. Without this kind of pass, it is hard to test
various cases in which nested `pop`s can be generated in existing
passes. This PR also adds `PassRegistry::registerTestPass`, which
registers a pass that's intended only for internal testing and does not
show up in `wasm-opt --help`.
Fixes #4237.
|
| |
|
|
|
| |
Check that types that were meant to have a subtype relationship actually do. To
expose the intended subtyping to the fuzzer, expose `subtypeIndices` in the
return value of the type generation function.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
As we work toward allowing nominal and structural types to coexist, any
difference in how they can be built or used will be an inconvenient footgun that
we will have to work around. In the spirit of reducing the differences between
the type systems, allow TypeBuilder to construct basic HeapTypes in nominal mode
just as it can in equirecursive mode.
Although this change is a net increase in code complexity for not much
benefit (wasm-opt never needs to build basic HeapTypes), it is also an
incremental step toward getting rid of separate type system modes, so I expect
it to simplify other PRs in the near future.
This change also uncovered a bug in how the type fuzzer generated subtypes of
basic HeapTypes. The generated subtypes did not necessarily have the intended
`Kind`, which caused failures in nominal subtype validation in the fuzzer.
|
| |
|
|
|
|
|
| |
- Do not require defaultable types in function returns
- Increase likelihood of `none` function return types
- Correctly generate subtypes of basic types
- Actually check output in tests
- Print to cout instead of cerr
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Add a new fuzzer binary that repeatedly generates random types to find bugs in
the type system implementation. Each iteration creates some number of root types
followed by some number of subtypes thereof. Each built type can contain
arbitrary references to other built types, regardless of their order of
construction.
Right now the fuzzer only finds fatal errors in type building (and in its own
implementation), but it is meant to be extended to check other properties in the
future, such as that LUB calculations work as expected.
The logic for creating types is also intended to be integrated into the main
fuzzer in a follow-on PR so that the main fuzzer can fuzz with arbitrarily more
interesting GC types.
|
| |
|
|
|
| |
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.
|
| |
|
|
| |
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.
|
| |
|
|
|
|
| |
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.
|
| |
|
|
|
|
| |
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.
|
| | |
|
| |
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
|
| |
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.
|
| | |
|
| | |
|
| |
|
|
| |
Adds the part of the spec test suite that this passes (without table.set we
can't do it all).
|
| |
|
|
|
| |
Locally I saw a 10% speedup on j2cl but reports of regressions have
arrived, so let's disable it for now pending investigation. The option added
here should make it easy to experiment.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Previously the set of functions to keep was initially empty, then the profile
added new functions to keep, then the --keep-funcs functions were added, then
the --split-funcs functions were removed. This method of composing these
different options was arbitrary and not necessarily intuitive, and it prevented
reasonable workflows from working. For example, providing only a --split-funcs
list would result in all functions being split out not matter which functions
were listed.
To make the behavior of these options, and --split-funcs in particular, more
intuitive, disallow mixing them and when --split-funcs is used, split out only
the listed functions.
|
| | |
|
| | |
|
| |
|
|
|
|
| |
We can assume that imported memories (and the profiling data they contain) are
already accessible from the module's environment, so there's no need to export
them. This also avoids needing to add knowledge of "profile-memory" to
Emscripten's library_dylink.js.
|
| |
|
| |
To support CMake 3.10. `add_executable` does not support OBJECT libraries until 3.12.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
To avoid requiring a static memory allocation, wasm-split's instrumentation
defaults to recording profile data in Wasm globals. This causes problems for
multithreaded applications because the globals are thread-local, but it is not
always feasible to arrange for a separate profile to be dumped on each thread.
To simplify the profiling of such multithreaded applications, add a new
instrumentation mode that stores the profiling data in shared memory instead of
in globals. This allows a single profile to be written that correctly reflects
the called functions on all threads.
This new mode is not on by default because it requires users to ensure that the
program will not trample the in-memory profiling data. The data is stored
beginning at address zero and occupies one byte per declared function in the
instrumented module. Emscripten can be told to leave this memory free using the
GLOBAL_BASE option.
|
| |
|
|
|
| |
As wasm-split has gained new functionality, its implementation file has become
large. In preparation for adding even more functionality, split the existing
implementation across multiple files in a new tools/wasm-split subdirectory.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The goal of this mode is to remove obviously-unneeded code like
(drop
(i32.load
(local.get $x)))
In general we can't remove it, as the load might trap - we'd be removing
a side effect. This is fairly rare in general, but actually becomes quite
annoying with wasm GC code where such patterns are more common,
and we really need to remove them.
Historically the IgnoreImplicitTraps option was meant to help here. However,
in practice it did not quite work well enough for most production code, as
mentioned e.g. in #3934 . TrapsNeverHappen mode is an attempt to fix that,
based on feedback from @askeksa in that issue, and also I believe this
implements an idea that @fitzgen mentioned a while ago (sorry, I can't
remember where exactly...). So I'm hopeful this will be generally useful
and not just for GC.
The idea in TrapsNeverHappen mode is that traps are assumed to not
actually happen at runtime. That is, if there is a trap in the code, it will
not be reached, or if it is reached then it will not trap. For example, an
(unreachable) would be assumed to never be reached, which means
that the optimizer can remove it and any code that executes right before
it:
(if
(..condition..)
(block
(..code that can be removed, if it does not branch out..)
(..code that can be removed, if it does not branch out..)
(..code that can be removed, if it does not branch out..)
(unreachable)))
And something like a load from memory is assumed to not trap, etc.,
which in particular would let us remove that dropped load from earlier.
This mode should be usable in production builds with assertions
disabled, if traps are seen as failing assertions. That might not be true
of all release builds (maybe some use traps for other purposes), but
hopefully in some. That is, if traps are like assertions, then enabling
this new mode would be like disabling assertions in release builds
and living with the fact that if an assertion would have been hit then
that is "undefined behavior" and the optimizer might have removed
the trap or done something weird.
TrapsNeverHappen (TNH) is different from IgnoreImplicitTraps (IIT).
The old IIT mode would just ignore traps when computing effects.
That is a simple model, but a problem happens with a trap behind
a condition, like this:
if (x != 0) foo(1 / x);
We won't trap on integer division by zero here only because of the
guarding if. In IIT, we'd compute no side effects on 1 / x, and then
we might end up moving it around, depending on other code in
the area, and potentially out of the if - which would make it happen
unconditionally, which would break.
TNH avoids that problem because it does not simply ignore traps.
Instead, there is a new hasUnremovableSideEffects() method
that must be opted-in by passes. That checks if there are no side
effects, or if there are, if we can remove them - and we know we can
remove a trap if we are running under TrapsNeverHappen mode,
as the trap won't happen by assumption. A pass must only use that
method where it is safe, that is, where it would either remove the
side effect (in which case, no problem), or if not, that it at least does
not move it around (avoiding the above problem with IIT).
This PR does not implement all optimizations possible with
TNH, just a small initial set of things to get started. It is already
useful on wasm GC code, including being as good as IIT on removing
unnecessary casts in some cases, see the test suite updates here.
Also, a significant part of the 18% speedup measured in
#4052 (comment)
is due to my testing with this enabled, as otherwise the devirtualization
there leaves a lot of unneeded code.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Technically this is not a new pass, but it is a rewrite almost from scratch.
Local Common Subexpression Elimination looks for repeated patterns,
stuff like this:
x = (a + b) + c
y = a + b
=>
temp = a + b
x = temp + c
y = temp
The old pass worked on flat IR, which is inefficient, and was overly
complicated because of that. The new pass uses a new algorithm that
I think is pretty simple, see the detailed comment at the top.
This keeps the pass enabled only in -O4, like before - right after
flattening the IR. That is to make this as minimal a change as possible.
Followups will enable the pass in the main pipeline, that is, we will
finally be able to run it by default. (Note that to make the pass work
well after flatten, an extra simplify-locals is added - the old pass used
to do part of simplify-locals internally, which was one source of
complexity. Even so, some of the -O4 tests have changes, due to
minor factors - they are just minor orderings etc., which can be
seen by inspecting the outputs before and after using e.g.
--metrics)
This plus some followup work leads to large wins on wasm GC output.
On j2cl there is a common pattern of repeated struct.gets, so common
that this pass removes 85% of all struct.gets, which makes the total
binary 15% smaller. However, on LLVM-emitted code the benefit is
minor, less than 1%.
|
| |
|
|
|
| |
Use ToolOptions there, which adds --nominal support.
We must also pass --nominal to the sub-commands we run.
|
| | |
|
| |
|
|
|
|
| |
It is ok to use the default value of a reference even if we refine the type,
as it would be a more specifically-typed null, and all nulls compare the
same. However, if the default is used then we *cannot* alter the type to
be non-nullable, as then we'd use a null where that is not allowed.
|
| |
|
|
|
|
|
|
|
| |
Practically NFC, but it does reorder some code a little. Previously we would
find a "zero", then shrink segments, then use that zero - which might no
longer be in the table. That seems weird, so this reorders that, but there
should be no significant difference in the output.
Also reduce the factor of 100 to 1, which in practice is important on one of
the Dart GC benchmarks that has a huge number of table segments.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
tryToRemoveFunctions (#4013)
tryToRemoveFunctions() will reload the wasm from binary if it fails to
optimize, and without the names section we don't have a guarantee on the
names being the same after that. And then tryToEmptyFunctions would
look for a name, and crash.
In the reverse order there is no risk, as tryToEmptyFunctions does not
reload the wasm from binary, it carefully undoes what it tried to do when
it fails.
|
| |
|
|
|
|
| |
Instead of skipping to the end, move quickly towards the end. This is
sometimes more efficient (as jumping from a big factor to a factor of 1
can skip over big opportunities to remove code all at once instead of
once instruction at a time).
|
| |
|
|
|
|
|
|
|
| |
This removes the code that did so one at a time, and instead adds it
in a way that we can do it in an exponentially growing set of functions.
On large testcases where other methods do not work, this is very
useful.
Also adjust the factor to do this 20x more often, which in practice
is very useful too.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
As suggested in
https://github.com/WebAssembly/binaryen/pull/3955#issuecomment-871016647
This applies commandline features first. If the features section is present, and
disallows some of them, then we warn. Otherwise, the features can combine
(for example, a wasm may enable feature X because it has to use it, and a user
can simply add the flag for feature Y if they want the optimizer to try to use it;
both flags will then be enabled).
This is important because in some cases we need to know the features before
parsing the wasm, in the case that the wasm does not use the features section.
In particular, non-nullable GC locals have an effect during parsing. (Typed
function references also does, but we found a way to apply its effect all the time,
that is, always use the refined type, and that happened to not break the case
where the feature is disabled - but such a workaround is not possible with
non-nullable locals.)
To make this less error-prone, add a FeatureSet input as a parameter to
WasmBinaryBuilder. That is, when building a module, we must give it the
features to use while doing so.
This will unblock #3955 . That PR will also add a test for the actual usage
of a feature during loading (the test can only be added there, after that PR
unbreaks things).
|
| |
|
|
|
|
|
|
|
| |
When using nominal types, func.ref of two functions with identical signatures
but different HeapTypes will yield different types. To preserve these semantics,
Functions need to track their HeapTypes, not just their Signatures.
This PR replaces the Signature field in Function with a HeapType field and adds
new utility methods to make it almost as simple to update and query the function
HeapType as it was to update and query the Function Signature.
|
| |
|
|
|
|
|
|
|
| |
This removes `attribute` field from `Tag` class, making the reserved and
unused field known only to binary encoder and decoder. This also removes
the `attribute` parameter from `makeTag` and `addTag` methods in
wasm-builder.h, C API, and Binaryen JS API.
Suggested in
https://github.com/WebAssembly/binaryen/pull/3946#pullrequestreview-687756523.
|
| |
|
|
|
|
|
|
|
|
|
| |
We recently decided to change 'event' to 'tag', and to 'event section'
to 'tag section', out of the rationale that the section contains a
generalized tag that references a type, which may be used for something
other than exceptions, and the name 'event' can be confusing in the web
context.
See
- https://github.com/WebAssembly/exception-handling/issues/159#issuecomment-857910130
- https://github.com/WebAssembly/exception-handling/pull/161
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Assertions were previously parsed by replacing "invoke" with "call" and using
the normal s-expr parser. The parseCall method of the s-expr parser uses the
call target to look up the correct signature on the module, but the invoke
targets in assertions use export names rather than internal function names, so
the signature lookups were inserting new bogus entries with default values.
This issue didn't seem to cause any big problems before, but #3935 turns it into
a hard error because the default `HeapType` does not have an associated
signature.
Fix the problem (at least in the common case of trivial arguments and expected
results) by manually construction a `Call` expression rather than depending on
the s-expr parser to construct it.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
This adds a new feature flag, GCNNLocals that enables support for
non-nullable locals. No validation is applied to check that they are
actually assigned before their use yet - this just allows experimentation
to begin.
This feature is not enabled by default even with -all. If we enabled it,
then it would take effect in most of our tests and likely confuse current
users as well. Instead, the flag must be opted in explicitly using
--enable-gc-nn-locals. That is, this is an experimental feature flag,
and as such must be explicitly enabled. (Once the spec stabilizes,
we will remove the feature anyhow when we implement the
final status of non-nullability. )
|
| |
|
|
|
|
|
| |
Adds a `--nominal` option to switch the type machinery from equirecursive to
nominal. Implements binary and text parsing and emitting of nominal types using
new type constructor opcodes and an `(extends $super)` text syntax extension.
When not in nominal mode, these extensions will still be parsed but will not
have any effect and will not be used when emitting.
|
| |
|
|
|
|
| |
The new instruction emits a file containing a map between placeholder index and
the name of the split out function that placeholder is replacing in the table.
This map is intended to be useful for debugging, as discussed in
https://github.com/emscripten-core/emscripten/issues/14330.
|
| |
|
|
|
|
|
| |
Given a list of profiles for the same module, --merge-profiles produces a single
combined profile the contains the minimum timestamp among the original profiles
for each function. When verbose output is enabled, also emit a message for each
profile that could individually be removed without affecting the set of
functions in the combined profile, as suggested in #3912.
|
| |
|
|
|
|
| |
In anticipation of adding a third wasm-split mode, merge-profiles, in addition
to the existing split and instrument modes, refactor wasm-split's option
validation to let the valid modes be declared for each option. This approach is
more scalable and robust than the ad-hoc validation we had previously.
|
