[clang] [llvm] [Coroutines] Mark parameter allocas with coro.outside.frame metadata (PR #127653)

[Hans Wennborg via cfe-commits]

zmodem wrote:

The discussion seems to be stalling. I'll try to summarize.

There seem to be two major questions:

1. Whether the front-end should explicitly mark some allocas as not belonging in the coroutine frame, or whether we should enhance CoroSplit to infer it automatically.

   I think explicitly marking the allocas is the best solution for the current bug (https://github.com/llvm/llvm-project/issues/127499), as well as the previous bug (https://github.com/llvm/llvm-project/issues/49843) which already adopted this approach.

   Based on @efriedma-quic's most recent comment,  I think he's also in favor of this approach(?):

   > So it's not really a question of "improving" the existing algorithm; we need markers in the IR, like coro_outside_frame, and the algorithm should be based on that.

   I don't think @rnk expressed an opinion on this question yet.

   @ChuanqiXu9 has argued in favor of the approach from https://github.com/llvm/llvm-project/pull/127524 instead: making CoroSplit treat allocas referenced after the `coro.end` intrinsic as belonging outside the frame.

2. If we do use explicit markers for the allocas, how should that be done?

   Currently, `coro.outside.frame` metadata on the allocas is used for this.

   @efriedma-quic pointed out metadata must never be required for correctness, and we all agree this current state is bad.

   I've proposed using an intrinsic.

   @rnk has suggested that we could put an explicit field on the alloca instruction, similar to `inalloca`.

   @ChuanqiXu9 has raised concerns about an intrinsic blocking optimization.

---

I'm keen to unbreak our users here, as well as implementing the proper long-term solution. Since there is some disagreement, maybe we shouldn't do it all at once.

I've rolled back my PR to just use the existing metadata. While that's not a complete solution, it should be a strict improvement: it fixes the immediate issue using the existing mechanism. I'd like to go ahead and land this.

I will follow up with a PR to remove the metadata, and we can continue discussing there.

https://github.com/llvm/llvm-project/pull/127653