Optimization PR and commits #6
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PassOptions is a fairly large structure and even includes a std::map. I also have plans to add further fields there to make it even larger. Before doing that I noticed that in some places we copy it instead of being consistent and taking it by reference, which this PR fixes.
Previously this pass would see something like this and fail:
if (foo() + global) {
global = 1;
}
The call to foo() has side effects, so we did not optimize. However, in such
a case the side effects are safe: they happen anyhow, regardless of the global
that we are optimizing. That is, "global" is read only to be written, even though
other things also influence the decision to write it. But "global" is not used in a
way that is observable: we can remove it, and nothing will notice (except for
things getting smaller/faster).
In other words, this PR will let us optimize the above example, while it also
needs to avoid optimizing the dangerous cases, like this:
if (foo(global)) {
global = 1;
}
Here "global" flows into a place that notices its value and may use it aside
from deciding to write that global.
A common case where we want to optimize is combined ifs,
if (foo()) {
if (global) {
global = 1;
}
}
which the optimizer turns into
if (foo() & global) {
global = 1;
}
With this PR we can handle those things too.
This lets us optimize out some important globals in j2wasm like the initializer
boolean for the Math object, reducing some total 0.5% of code size.
…t 3) (#4338) (i32(x) < 0) | (i32(y) < 0) ==> i32(x | y) < 0 (i32(x) != 0) | (i32(y) != 0) ==> i32(x | y) != 0 Likewise for i64.
This adds support for try-delegate in `EffectAnalyzer`. Without this
support, the expresion below has been incorrectly classified as "cannot
throw", because the previous code considered everything inside
`try`-`catch_all` as "cannot throw". This is not the case when there is
a `delegate` that can bypass the `catch_all`.
```wasm
try $l0
try
try
throw $e
delegate $l0
catch_all
end
end
All EH tests in test/lit/passes currently have the suffix `-eh`, so I think it's better be consistent for this one.
We do some postprocessing after parsing `Try` to make sure `delegate` only targets `try`s and not `block`s: https://github.com/WebAssembly/binaryen/blob/9659f9b07c1196447edee68fe04c8d7dd2480652/src/wasm/wasm-binary.cpp#L6404-L6426 But in case the outer `try` has neither of `catch` nor `delegate`, the previous code just return prematurely, skipping the postprocessing part, resulting in a binary parsing error. This PR removes that early-exiting code. Some test outputs have changed because `try`s are assigned labels after the early exit. But those labels can be removed by other optimization passes when there is no inner `rethrow` or `delegate` that targets them. (On a side note, the restriction that `delegate` cannot target a `block` has been removed a few months ago in the spec, so if a `delegate` targets a `block`, it means it is just rethrown from that block. But I still think this is a convenient invariant to hold at least within the binaryen IR. I'm planning to allow parsing of `delegate` targeting `block`s later, but I will make them point to `try` when read in the IR. At the moment the LLVM toolchain does not generate such code.)
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.
Types and HeapTypes are inserted into their respective stores either by copying a reference to a `TypeInfo` or `HeapTypeInfo` or by moving a `std::unique_ptr<TypeInfo>` or `std::unique_ptr<HeapTypeInfo>`. Previously these two code paths had separate, similar logic. To reduce deduplication, combine both code paths into a single method.
We don't use those effects now in any way, and if we need them some day we can add them back. For now they just add overhead and complexity.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WebAssembly#4265
PR list of optimization for fuzzing and PR management