Repository navigation
[Wasm GC] Optimize BrOn* of the wrong kind - #3724
Merged
Merged
Conversation
aheejin
approved these changes
Mar 24, 2021
Comment on lines
+392
to
+395
| // (Note: if the spec had BrOnNonNull, instead of BrOnNull, then we could | ||
| // replace a br_on_func whose input is (ref null func) with br_on_non_null, | ||
| // as only the null check would be needed. But as things are, we cannot do | ||
| // such a thing.) |
Member
There was a problem hiding this comment.
Aren't both of them (br_on_func and hypothetical br_on_non_null) conditional branches anyway? Is there a benefit in converting one to the other?
Member
Author
There was a problem hiding this comment.
Both are conditional branches, yes, but in some VMs a null check might not need to load the value (it can just see the pointer is null). That might not be most VMs, though... but even in other VMs a null check is more common, and so perhaps more likely to be folded away in the optimizing compiler (if there's another null check before it anyhow).
A definite advantage though is that GC operations have a byte prefix. br_on_null and a hypothetical br_on_non_null could be one byte, while br_on_func is two.
kripken
added a commit
that referenced
this pull request
Mar 30, 2021
This is similar to the optimization of BrOn in #3719 and #3724. When the type tells us the kind of input we have, we can tell at compile time what result we'll get, like ref.is_func of something with type (ref func) will always return 1, etc. There are some code size and perf tradeoffs that should be looked into more and are marked as TODOs.
martianboy
pushed a commit
to martianboy/binaryen
that referenced
this pull request
Mar 31, 2021
…#3725) This is similar to the optimization of BrOn in WebAssembly#3719 and WebAssembly#3724. When the type tells us the kind of input we have, we can tell at compile time what result we'll get, like ref.is_func of something with type (ref func) will always return 1, etc. There are some code size and perf tradeoffs that should be looked into more and are marked as TODOs.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
#3719 optimized the case where the kind is what we want, like
br_on_funcofa function. This handles the opposite case, where we know the kind is wrong, and
so the break is not taken.
This seems equally useful for "polymorphic" code that does a bunch of checks
and routes to different code for each case, as after inlining or other optimizations
we may see which paths are taken and which are not.
Also refactor the checking code to a shared location as RefIs/As will also want to
use it.