[llvm] [coroutines][CoroSplit] Store allocas on the frame if there is no explicit lifetime.end (PR #88806)

Alan Zhao via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 18 00:21:08 PDT 2024


alanzhao1 wrote:

> > Is someone looking at doing the deeper fix to the lifetime analysis as suggested above, or should we do a stop gap fix in the meantime?
> 
> I have less time developing into coroutines in detail recently. So if there is no one had the chance to fix this properly in the near future, I think we can accept this as a partial workaround.
> 
> > I'm not sure if the current patch is sufficient: it checks whether there is a lifetime.end intrinsic, but what if there is one on one execution path, but not another?
> 
> Yeah, this is the reason why I think the current patch is a partial fix.
 
Agreed that this is a partial fix - it doesn't address the example that @zmodem mentioned, plus there could be other scenarios that we didn't think of. A complete fix would probably require an RFC on Discourse due to its complexity (llvm lifetimes intrinsics are notoriously fickle per https://github.com/llvm/llvm-project/pull/88806#issuecomment-2058249478, and we'd likely have to rewrite most of the existing logic here.)

> > Maybe we should always set ShouldUseLifetimeStartInfo to avoid this miscompile for now?
> 
> I am not sure what you mean. `ShouldUseLifetimeStartInfo` is always set for C++.

I think the proposal is that we put all coroutine variables on the coroutine frame object rather than try to figure out what variables can go on the stack. This would generate suboptimal code, but it will be guaranteed to be correct.

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


More information about the llvm-commits mailing list