[PATCH] D94002: [LangRef] Make lifetime intrinsic's semantics consistent with StackColoring's comment

[Nuno Lopes via Phabricator via llvm-commits]

nlopes added inline comments.


================
Comment at: llvm/docs/LangRef.rst:17871
+If ``ptr`` is a stack-allocated object and its offset is zero, the object is
+initially marked as dead.
+After '``llvm.lifetime.start``', the stack object that ``ptr`` points is marked
----------------
RalfJung wrote:
> This sentence here is doing a lot of work. In particular, it makes the behavior of `alloca` *depend on the future of the current execution*, i.e. behavior depends on whether at some point in the future, a lifetime.start intrinsic is called on the resulting pointer. I think such acausal definitions are a mistake; they make it impossible to write an interpreter that just accurately executes LLVM IR and detects UB.
> 
> At the very least, the documentation of `alloca` should be adjusted to explicitly talk about this. Right now, the documentation of lifetime.start alters the behavior of another instruction, which is really surprising and will easily be missed.
> 
> Does LLVM correctly handle code like the following, which should not be UB even though there is an access to an alloca before the lifetime.start? (Imagine this to be LLVM IR code, and without "mustprogress")
> 
> ```
> x = alloca ...;
> *x = 5;
> while (true) {}
> lifetime.start(x)
> ```
Ralf we already agreed that the current design is suboptimal and that it makes it impossible to write a *precise* interpreter for LLVM IR.
The exact semantics of lifetime.start depends on the pattern matching patterns in the stack coloring algorithm. So this intrinsic cannot be abused. It must be used for the uses it was created for only.
We agree a better design would be to tag allocas with `dead` so they are initially dead or just to allow allocas in any BB rather than having all them pushed to the first BB (if we assume a basic stack allocation algorithm would run even in fastisel).

So while we can write a billion corner-case examples, by hand, they are not useful for the discussion of this particular patch. We can't go back in time and fix the current design. We can't change the current design without breaking all the frontends out there either.
The way forward, if someone is interested in doing more aggressive optimizations, is to design a new mechanism that would have to live side-by-side with the current one for a couple of years to allow frontends to migrate. Or create an auto-upgrade algorithm to convert the old idiom to the new one.

I agree adding a note to alloca and cross-reference this section makes sense.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94002/new/

https://reviews.llvm.org/D94002