[PATCH] D99135: [deref] Implement initial set of inference rules for deref-at-point

[Nuno Lopes via Phabricator via llvm-commits]

nlopes added inline comments.


================
Comment at: llvm/lib/IR/Value.cpp:741
+  // *) An alloca is deallocated when the function returns
+  if (isa<GlobalValue>(V) || isa<Constant>(V) || isa<AllocaInst>(V))
+    return false;
----------------
reames wrote:
> reames wrote:
> > nlopes wrote:
> > > an Alloca can be killed with the lifetime intrinsics. Storing to an alloca after lifetime_end is UB.
> > Right, but how does that affect dereferenceability?  The memory is still dereferenceable.  The contents may be undefined, but the access won't fault.
> > 
> > p.s. Lifetime intrinsics are very badly specified, and are inconsistent across the optimizer - including in the existing deref code.  I really don't want them to be a blocker here.  
> JFYI, the wording in the LangRef on semantics of a dead object (after lifetime end) was recently changed in c821ef4.  I objected to the wording at the time, though the patch appears not to have been adjusted.  This is why.  
> 
> lifetime intrinsics do not change dereferenceability.  The relevant wording from c821ef4 needs removed from the LangRef.  It is wrong.  Both with current and proposed semantics.
I agree that doing a store after lifetime_end intrinsic doesn't cause a crash (ATM), but it may write to another object.
For example:
  %p = alloca i8
  %q = alloca i8
  
  lifetime_start %p
  ...
  lifetime_end %p
  lifetime_start %q
  store i8 1, %q
  store i8 0, %p ; UB: %p is dead
  
  %v = load i8, %q

What's the value of %v? In practice, it's going to be 0 because %p & %q will end up allocated at the same address. And this is why storing after lifetime_end must be UB.

So now it depends on how this analysis is used. My concern is that it may be used to move a store past lifetime_end and then we have a miscompilation. Because if a pointer is dereferenceable for the size of the store and it's guaranteed to be alive throughout the function, then  we can conclude that loads & stores to that pointer can be moved freely.

A simple solution is to check if the particular alloca is an argument to a lifetime_end intrinsic. More complex ones require an additional argument like a BB or specific instruction, such that the query becomes: is the object still alive at this BB/instruction?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99135