Andres-Salamanca wrote:

> Jumping in to bump this a bit, since the suspend-point discussion has 
> stretched across a few weeks.
> ### On the direction (discussion about the suspend point)
> 
> +1 to landing on the cleanup-style abstraction. The key thing I'd add, since 
> you got stuck on "reference to block defined in another region" when you 
> tried a cir.br to a CodeGen-created block: with the cleanup framing you don't 
> name the destination at all. The await's suspend region should end in a 
> structured terminator (a yield / cir.coro.suspend-style exit), and FlattenCFG 
> wires that exit to the enclosing continuation exactly like it already wires 
> cir.yield out of a scope/cleanup region. This PR already computes that 
> continuation in CIRCoroBodyOpFlattening (for co_return) and already taught 
> CIRCleanupScopeOpFlattening to thread CoReturnOp, so the suspend path should 
> reach that same continuation through the structural nesting. No beacon, no 
> cross-region branch, and the destination falls out of flattening for free. 
> That should remove the wall you hit.
> ### On making progress now
> 
> This is redesign work, so consider splitting the PR. The coro.body/co_return 
> flattening, the intrinsic emission, and the test reorg look largely 
> independent and landable on their own; the await suspend-destination piece is 
> where the redesign lives. That lets you land the uncontroversial 80% while we 
> iterate on the suspend representation.
> 
> Independent of the redesign, three small open items worth closing out 
> regardless:
> 
>     * andykaylor's missing-suspend-point-alloca error handling still needs a 
> response.
> 
>     * his "do you need to flatten nested regions before this?" question is 
> unanswered.
> 
>     * the IR-to-IR test suggestion (checking the pre/post-flatten CIR 
> directly) hasn't been picked up, that'd make this much easier to review than 
> the .cpp-driven test.
> 
> 
> @Andres-Salamanca: Thanks for pushing on this!! Coroutine flattening is 
> gnarly and the structured-CFG constraints make it more so.

@andykaylor @bcardosolopes

Thanks for the summary! Yes, I've also been working on the GRO manager 
representation in the IR, which needs flattening as well, and looking into the 
suspend-point redesign. Because of that, I haven't spent much time on this PR 
yet.

Per Bruno's suggestion, I'll scope this PR down to just the coro.body and 
co_return flattening, land that part, and reorganize the tests accordingly. The 
intrinsic emission, GRO manager, and suspend-point redesign will move into 
separate PRs.

Does this plan sound right to you both?


https://github.com/llvm/llvm-project/pull/203802
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to