Differential D110953
[coro async] Disable lifetime.start sinking for ABI::Async and ABI::Retcon Authored by aschwaighofer on Oct 1 2021, 11:34 AM.
Details It does not handle loops correctly i.e it moves the lifetime.start entry: %obj = alloca i8 lifetime.start(%obj) br loop loop: coro.suspend() escape(%obj) cond_br %cond, label %exit, label loop br loop exit: lifetime.end(%obj After sinking: entry: %obj = alloca i8 br loop loop: coro.suspend() lifetime.start(%obj) escape(%obj) cond_br %cond, label %exit, label loop br loop exit: lifetime.end(%obj rdar://83411917
Diff Detail
Event TimelineComment Actions What's the justification for leaving this on for the switch lowering? It sounds like it's a broken optimization that could trigger in any sort of coroutine. Comment Actions @rjmccall I am not familiar with (the possible constraints of/assumptions) switch lowering but I would assume that it is broken there too (I don't have the time to investigate so I did not want to regress them needlessly). Comment Actions @lxfind What do you think, is that scenario also possible with Switch lowering. Should we disable lifetime sinking and lifetime based optimization (https://reviews.llvm.org/D110949) for switch lowering until it is fixed? An example like the one mentioned in the commit message will fail after we have sunk the lifetime.start into the loop because the check whether we cross a suspend by starting at the lifetime.begin will fail leading us to use a localized alloca rather than a frame entry. Another way of thinking about it is that by sinking the lifetime.start we are saying that the memory location is unitialized between the lifetime.start and the first write to its location (%obj). That is definitely not true along the path through the back-edge (other optimizations could mis-optimize based on this). entry: %obj = alloca i8 lifetime.start(%obj) // <== sink from here br loop loop: coro.suspend() lifetime.start(%obj) // <== to here escape(%obj) cond_br %cond, label %exit, label loop exit: lifetime.end(%obj) In terms of IR constrains it seems we are violating this constraint (https://llvm.org/docs/LangRef.html#llvm-lifetime-start-intrinsic) by performing the sinking in the example listed above:
|