Optimize resume of non-suspending continuations - #9071
Conversation
This will be necessary for a future optimization that turns resumes of continuations that never suspend into calls. Update the effect analysis of suspends to set the new effect and clobber global state because the suspend handler might do anything before returning. Test that the effects are analyzed as intended and that they work with global effect analysis.
Add a `visitResume` in OptimizeInstructions that does the normal optimizations on null continuations and then tries to turn resumes into calls. We can do this when we are resuming a freshly allocated continuation created with a reference to a known function that GlobalEffects tells us will not suspend.
| // If the continuation expression has no side effects, we can eliminate it | ||
| // entirely and replace the resume with a direct call. | ||
| if (!effects(curr->cont).hasSideEffects()) { | ||
| replaceCurrent( | ||
| builder.makeCall(target->name, curr->operands, target->getResults())); | ||
| return; | ||
| } | ||
|
|
||
| // The continuation expression has side effects. In Wasm, resume operands | ||
| // are evaluated before the continuation expression. If there are no | ||
| // operands, evaluate the continuation (dropped) and then call. | ||
| if (curr->operands.empty()) { | ||
| replaceCurrent(builder.makeSequence( | ||
| builder.makeDrop(curr->cont), | ||
| builder.makeCall(target->name, {}, target->getResults()))); | ||
| return; | ||
| } | ||
|
|
||
| // In the presence of operands, execute the code in curr->cont after the | ||
| // operands and before the call happens by spilling the last operand to a | ||
| // temporary local. | ||
| auto* lastOperand = curr->operands.back(); | ||
| auto lastOperandType = lastOperand->type; | ||
| Index tempLocal = builder.addVar(getFunction(), lastOperandType); | ||
| auto* set = builder.makeLocalSet(tempLocal, lastOperand); | ||
| auto* drop = builder.makeDrop(curr->cont); | ||
| auto* get = builder.makeLocalGet(tempLocal, lastOperandType); | ||
| curr->operands.back() = builder.makeBlock({set, drop, get}); | ||
| replaceCurrent( | ||
| builder.makeCall(target->name, curr->operands, target->getResults())); | ||
| } |
There was a problem hiding this comment.
Can we use ChildLocalizer to simplify all this code 1514-1544?
That is, put all the operands of curr in locals, then make a block that ends with a call?
There was a problem hiding this comment.
I don't think so. ChildLocalizer will remove children without side effects, but we're trying to hook up the children to a new parent.
There was a problem hiding this comment.
It doesn't remove children, it puts them into locals if they have effects. After it runs, the updated children are simple things without effects, which can be moved.
After running ChildLocalizer on the cont.new that has the children we want to move, we can then simply copy the updated children to the new place. We will be copying things like local.gets.
There was a problem hiding this comment.
Ah, right. That sounds like it would work. Thanks!
There was a problem hiding this comment.
This is from the other PR I think? Do stacked PRs not hide the shared parts?
There was a problem hiding this comment.
This isn't stacked as far as GH is concerned because I've been trying to keep using merges instead of rebases. I guess it didn't help in this case.
| } | ||
|
|
||
| auto* funcExpr = Properties::getFallthrough( | ||
| contNew->func, getPassOptions(), *getModule(), behavior); |
There was a problem hiding this comment.
| contNew->func, getPassOptions(), *getModule(), behavior); | |
| contNew->func, getPassOptions(), *getModule()); |
The fallthrough of the function doesn't need to care about other uses and traps and such.
|
|
||
| auto* block = | ||
| ChildLocalizer(curr, getFunction(), *getModule(), getPassOptions()) | ||
| .getChildrenReplacement(); |
There was a problem hiding this comment.
I realize that this is almost but not quite right. ChildLocalizer considers child effects wrt each other. After the operation, it is safe to reorder and remove them. But moving them across other effects might not be safe.
Unless we know no other effects can be in the middle, here? But it seems like there can be:
(resume
(OPERANDS)
(block
(STUFF A)
(cont.new
(block
(STUFF B)
(ref.func)
)
)
)
)
=>
(STUFF A)
(STUFF B)
(call
(OPERANDS)
)OPERANDS is moved past the STUFFs.
This is simple to handle, though: add a flag to ChildLocalizer to control this behavior. Where it computes
// Use a local if we need to. That is the case either if this has side
// effects we can't remove, or if it interacts with other children.then, we can have a mode where it uses a local for any child with effects, even removable ones.
There was a problem hiding this comment.
But ChildLocalizer sees all the children of the resume, including the one that has STUFF A and STUFF B here, so I think this is safe. See the $test-eval-order test, for example.
There was a problem hiding this comment.
Oh, right... all the last child (with STUFF) is, is just another child for ChildLocalizer. Nice it works out so well!
|
|
||
| auto* block = | ||
| ChildLocalizer(curr, getFunction(), *getModule(), getPassOptions()) | ||
| .getChildrenReplacement(); |
There was a problem hiding this comment.
Oh, right... all the last child (with STUFF) is, is just another child for ChildLocalizer. Nice it works out so well!
| ;; Test that optimize-instructions turns resumptions of continuations that never | ||
| ;; suspend into direct calls. | ||
|
|
||
| ;; RUN: wasm-opt %s --all-features --generate-global-effects --remove-unused-names --optimize-instructions -S -o - | filecheck %s --check-prefix NO-TNH |
There was a problem hiding this comment.
is removed-unused-names needed here and below?
| ;; VACUUM-NEXT: ) | ||
| ;; VACUUM-NEXT: ) | ||
| (func $test-eval-order-non-nullable (param $x (ref any)) (result (ref any)) | ||
| ;; Evaluation order test: non-nullable operands are preserved when spilled. |
There was a problem hiding this comment.
Which non-nullable operand is "spilled" here? I was guessing x but no local is added for it.
There was a problem hiding this comment.
Yeah, this test seems obsolete now that we're using ChildLocalizer. I've removed it.
2 participants
Add a
visitResumein OptimizeInstructions that does the normal optimizations on null continuations and then tries to turn resumes into calls. We can do this when we are resuming a freshly allocated continuation created with a reference to a known function that GlobalEffects tells us will not suspend.