-
Notifications
You must be signed in to change notification settings - Fork 833
[WasmGC] Heap2Local: Optimize RefEq #6703
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a4e1778
38f2297
3009f94
10a8b9d
43e5381
d36281b
1c11a1e
19823a6
078b813
090af2d
49d7545
60050be
dea6db5
9029ba2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -349,6 +349,12 @@ struct EscapeAnalyzer { | |||||||||||||
| void visitLocalSet(LocalSet* curr) { escapes = false; } | ||||||||||||||
|
|
||||||||||||||
| // Reference operations. TODO add more | ||||||||||||||
| void visitRefEq(RefEq* curr) { | ||||||||||||||
| // The reference is compared for identity, but nothing more. | ||||||||||||||
| escapes = false; | ||||||||||||||
| fullyConsumes = true; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| void visitRefAs(RefAs* curr) { | ||||||||||||||
| // TODO General OptimizeInstructions integration, that is, since we know | ||||||||||||||
| // that our allocation is what flows into this RefAs, we can | ||||||||||||||
|
|
@@ -507,14 +513,18 @@ struct EscapeAnalyzer { | |||||||||||||
| // efficient, but it would need to be more complex. | ||||||||||||||
| struct Struct2Local : PostWalker<Struct2Local> { | ||||||||||||||
| StructNew* allocation; | ||||||||||||||
| const EscapeAnalyzer& analyzer; | ||||||||||||||
|
|
||||||||||||||
| // The analyzer is not |const| because we update |analyzer.reached| as we go | ||||||||||||||
| // (see replaceCurrent, below). | ||||||||||||||
| EscapeAnalyzer& analyzer; | ||||||||||||||
|
|
||||||||||||||
| Function* func; | ||||||||||||||
| Module& wasm; | ||||||||||||||
| Builder builder; | ||||||||||||||
| const FieldList& fields; | ||||||||||||||
|
|
||||||||||||||
| Struct2Local(StructNew* allocation, | ||||||||||||||
| const EscapeAnalyzer& analyzer, | ||||||||||||||
| EscapeAnalyzer& analyzer, | ||||||||||||||
| Function* func, | ||||||||||||||
| Module& wasm) | ||||||||||||||
| : allocation(allocation), analyzer(analyzer), func(func), wasm(wasm), | ||||||||||||||
|
|
@@ -539,6 +549,15 @@ struct Struct2Local : PostWalker<Struct2Local> { | |||||||||||||
| // In rare cases we may need to refinalize, see below. | ||||||||||||||
| bool refinalize = false; | ||||||||||||||
|
|
||||||||||||||
| Expression* replaceCurrent(Expression* expression) { | ||||||||||||||
| PostWalker<Struct2Local>::replaceCurrent(expression); | ||||||||||||||
| // Also update |reached|: we are replacing something that was reached, so | ||||||||||||||
| // logically the replacement is also reached. This update is necessary if | ||||||||||||||
| // the parent of an expression cares about whether a child was reached. | ||||||||||||||
| analyzer.reached.insert(expression); | ||||||||||||||
| return expression; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Rewrite the code in visit* methods. The general approach taken is to | ||||||||||||||
| // replace the allocation with a null reference (which may require changing | ||||||||||||||
| // types in some places, like making a block return value nullable), and to | ||||||||||||||
|
|
@@ -688,6 +707,27 @@ struct Struct2Local : PostWalker<Struct2Local> { | |||||||||||||
| replaceCurrent(builder.makeBlock(contents)); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| void visitRefEq(RefEq* curr) { | ||||||||||||||
| if (!analyzer.reached.count(curr)) { | ||||||||||||||
| return; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| if (curr->type == Type::unreachable) { | ||||||||||||||
| // The result does not matter. Leave things as they are (and let DCE | ||||||||||||||
| // handle it). | ||||||||||||||
| return; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // If our reference is compared to itself, the result is 1. If it is | ||||||||||||||
| // compared to something else, the result must be 0, as our reference does | ||||||||||||||
| // not escape to any other place. | ||||||||||||||
| int32_t result = analyzer.reached.count(curr->left) > 0 && | ||||||||||||||
| analyzer.reached.count(curr->right) > 0; | ||||||||||||||
|
Comment on lines
+724
to
+725
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this checking whether the expression is compared to itself?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if one (or both) of the arms may or may not be the analyzed allocation?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are never in an ambiguous situation. If one arm can contain a mixture of the analyzed allocation and something else then we consider that the binaryen/src/passes/Heap2Local.cpp Lines 243 to 248 in 60050be
In theory we could add runtime checks "is this the allocation", but it seems unpromising. |
||||||||||||||
| // For simplicity, simply drop the RefEq and put a constant result after. | ||||||||||||||
| replaceCurrent(builder.makeSequence(builder.makeDrop(curr), | ||||||||||||||
| builder.makeConst(Literal(result)))); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| void visitRefAs(RefAs* curr) { | ||||||||||||||
| if (!analyzer.reached.count(curr)) { | ||||||||||||||
| return; | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to make sure I understand: we've done a data flow analysis and determined that this expression is not dead? Is it possible for
reachedto be nonzero and still have typeType::unreachablebelow?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yeah, "reach" is used in two ways here.
reachedmeans that the allocation, thestruct.new / array.new, reaches this location. So this first check is just making sure that we only look in the places we traced the path of the allocation.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider
That allocation reaches the
ref.eq, but also theref.eqis unreachable code, separately.