Typed continuations: suspend instructions #6393
Conversation
| I haven't been able to run the fuzzer on this, because it immediately reports a fault even on the main branch. Is that a known issue? I've seen that there's been quite a few changes to the fuzzer lately, so I was wondering if it's supposed to be usable at the moment? |
| Hmm, no, I think it's surprising that the fuzzer is failing on the main branch. |
| Is that with latest V8? Then that is a known issue due to the |
Thanks, using that fixed the problem! 👍 |
src/ir/cost.h Outdated
| return 12 + visit(curr->cont); | ||
| } | ||
| CostType visitSuspend(Suspend* curr) { | ||
| // Cheaper than resume, since payloads cannot be partially applied. |
There was a problem hiding this comment.
Can you explain this more? What does it mean that resume can partially apply payloads?
There was a problem hiding this comment.
This is definitely my most dreaded file now 😅
What I meant in the original comment here is the following: If you call resume on a continuation, it may have been produced with cont.bind, meaning that we have partially applied some arguments to the continuation, and resume only needs to provide the remaining ones. That means that we need to perform some runtime check that determines how to handle payloads in resume.
In contrast, suspend always sees all the payloads statically: When you call suspend, you provide all the payloads at once.
But there are more considerations when talking about the performance of suspend and resume, so mentioning only partial applications in my comment was a bit misleading: resume may have to check if you are initiating execution for the first time vs continuing after a suspension, suspend may need to forward to an outer handler, etc.
Given that suspend works in tandem with resume (i.e., whenever you call suspend you continue execution at another resume) and they both do stack switching, I'm now feeling more inclined to just give suspend the same cost as resume and delete my comment on suspend. Does that sound okay?
There was a problem hiding this comment.
Thanks, makes sense.
Yes, giving them the same cost without comment seems fine. Trying to be too precise in the reasoning here doesn't make sense given how imprecise all the numbers actually are :)
There was a problem hiding this comment.
Great, I've updated the function for suspend accordingly. I've also realized that I forgot to include the cost of the operands when calculating the cost of resume, so I fixed that in one go.
| void Suspend::finalize(Module* wasm) { | ||
| if (wasm) { | ||
| auto tag = wasm->getTag(this->tag); | ||
| type = tag->sig.results; | ||
| } | ||
| } | ||
| |
There was a problem hiding this comment.
This needs to set the type to Type::unreachable if any of the operands are unreachable, regardless of whether wasm is present or not.
(We should do this for the other instructions as well if we forgot to do this before)
There was a problem hiding this comment.
I've added these checks now, here and for the earlier WasmFX instructions. If that's what you had in mind I'll update the description of the PR to mention that it also fixes finalization of the other instructions.
| Yep, that last change looks good 👍 Let me know when you've updated the description and I'll merge this. |
| Great, I've updated the description now. Any thoughts on the question in |
| Oops, sorry, I had missed that. |
3 participants
This PR is part of a series that adds basic support for the typed continuations/wasmfx proposal.
This particular PR adds support for the
suspendinstruction for suspending with a given tag, documented here.These instructions are of the form
(suspend $tag). Assuming that$tagis defined with nparamtypest_1tot_n, the instruction consumes n arguments of typest_1tot_n. Its result type is the same as theresulttype of the tag.Thus, the folded textual representation looks like
(suspend $tag arg1 ... argn).Support for the instruction is implemented in both the old and the new wat parser.
Note that this PR does not implement validation of the new instruction.
This PR also fixes finalization of
cont.new,cont.bindandresumenodes in those cases where any of their children are unreachable.