Fix aliasing of launder.invariant.group

Summary:
Patch for capture tracking broke
bootstrap of clang with -fstict-vtable-pointers
which resulted in debbugging nightmare. It was fixed
https://reviews.llvm.org/D46900 but as it turned
out, there were other parts like inliner (computing of
noalias metadata) that I found after bootstraping with enabled
assertions.

Reviewers: hfinkel, rsmith, chandlerc, amharc, kuhar

Subscribers: JDevlieghere, eraman, llvm-commits, hiraditya

Differential Revision: https://reviews.llvm.org/D47088

llvm-svn: 333070

llvm/include/llvm/Analysis/ValueTracking.h

@@ -276,6 +276,22 @@ class Value;
   /// pointer, return 'len+1'.  If we can't, return 0.
   uint64_t GetStringLength(const Value *V, unsigned CharSize = 8);
 
+  /// This function returns call pointer argument that is considered the same by
+  /// aliasing rules. You CAN'T use it to replace one value with another.
+  const Value *getArgumentAliasingToReturnedPointer(ImmutableCallSite CS);
+  inline Value *getArgumentAliasingToReturnedPointer(CallSite CS) {
+    return const_cast<Value *>(
+        getArgumentAliasingToReturnedPointer(ImmutableCallSite(CS)));
+  }
+
+  // {launder,strip}.invariant.group returns pointer that aliases its argument,
+  // and it only captures pointer by returning it.
+  // These intrinsics are not marked as nocapture, because returning is
+  // considered as capture. The arguments are not marked as returned neither,
+  // because it would make it useless.
+  bool isIntrinsicReturningPointerAliasingArgumentWithoutCapturing(
+      ImmutableCallSite CS);
+
   /// This method strips off any GEP address adjustments and pointer casts from
   /// the specified value, returning the original object being addressed. Note
   /// that the returned value has pointer type if the specified value does. If

llvm/lib/Analysis/BasicAliasAnalysis.cpp

@@ -132,16 +132,8 @@ static bool isNonEscapingLocalObject(const Value *V) {
 /// Returns true if the pointer is one which would have been considered an
 /// escape by isNonEscapingLocalObject.
 static bool isEscapeSource(const Value *V) {
-  if (auto CS = ImmutableCallSite(V)) {
-    // launder_invariant_group captures its argument only by returning it,
-    // so it might not be considered an escape by isNonEscapingLocalObject.
-    // Note that adding similar special cases for intrinsics in CaptureTracking
-    // requires handling them here too.
-    if (CS.getIntrinsicID() == Intrinsic::launder_invariant_group)
-      return false;
-
+  if (ImmutableCallSite(V))
     return true;
-  }
 
   if (isa<Argument>(V))
     return true;
@@ -438,11 +430,19 @@ bool BasicAAResult::DecomposeGEPExpression(const Value *V,
 
     const GEPOperator *GEPOp = dyn_cast<GEPOperator>(Op);
     if (!GEPOp) {
-      if (auto CS = ImmutableCallSite(V))
-        if (const Value *RV = CS.getReturnedArgOperand()) {
-          V = RV;
+      if (auto CS = ImmutableCallSite(V)) {
+        // Note: getArgumentAliasingToReturnedPointer keeps it in sync with
+        // CaptureTracking, which is needed for correctness.  This is because
+        // some intrinsics like launder.invariant.group returns pointers that
+        // are aliasing it's argument, which is known to CaptureTracking.
+        // If AliasAnalysis does not use the same information, it could assume
+        // that pointer returned from launder does not alias it's argument
+        // because launder could not return it if the pointer was not captured.
+        if (auto *RP = getArgumentAliasingToReturnedPointer(CS)) {
+          V = RP;
           continue;
         }
+      }
 
       // If it's not a GEP, hand it off to SimplifyInstruction to see if it
       // can come up with something. This matches what GetUnderlyingObject does.

llvm/lib/Analysis/CaptureTracking.cpp

@@ -22,6 +22,7 @@
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/CFG.h"
 #include "llvm/Analysis/OrderedBasicBlock.h"
+#include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Dominators.h"
@@ -247,11 +248,12 @@ void llvm::PointerMayBeCaptured(const Value *V, CaptureTracker *Tracker) {
       if (CS.onlyReadsMemory() && CS.doesNotThrow() && I->getType()->isVoidTy())
         break;
 
-      // launder.invariant.group only captures pointer by returning it,
-      // so the pointer wasn't captured if returned pointer is not captured.
-      // Note that adding similar special cases for intrinsics requires handling
-      // them in 'isEscapeSource' in BasicAA.
-      if (CS.getIntrinsicID() == Intrinsic::launder_invariant_group) {
+      // The pointer is not captured if returned pointer is not captured.
+      // NOTE: CaptureTracking users should not assume that only functions
+      // marked with nocapture do not capture. This means that places like
+      // GetUnderlyingObject in ValueTracking or DecomposeGEPExpression
+      // in BasicAA also need to know about this property.
+      if (isIntrinsicReturningPointerAliasingArgumentWithoutCapturing(CS)) {
         AddUses(I);
         break;
       }

llvm/lib/Analysis/Loads.cpp

@@ -107,14 +107,11 @@ static bool isDereferenceableAndAlignedPointer(
     return isDereferenceableAndAlignedPointer(ASC->getOperand(0), Align, Size,
                                               DL, CtxI, DT, Visited);
 
-  if (auto CS = ImmutableCallSite(V)) {
-    if (const Value *RV = CS.getReturnedArgOperand())
-      return isDereferenceableAndAlignedPointer(RV, Align, Size, DL, CtxI, DT,
+  if (auto CS = ImmutableCallSite(V))
+    if (auto *RP = getArgumentAliasingToReturnedPointer(CS))
+      return isDereferenceableAndAlignedPointer(RP, Align, Size, DL, CtxI, DT,
                                                 Visited);
-    if (CS.getIntrinsicID() == Intrinsic::launder_invariant_group)
-      return isDereferenceableAndAlignedPointer(CS->getOperand(0), Align, Size,
-                                                DL, CtxI, DT, Visited);
-  }
+
   // If we don't know, assume the worst.
   return false;
 }

llvm/lib/Analysis/ValueTracking.cpp

@@ -1956,8 +1956,8 @@ bool isKnownNonZero(const Value *V, unsigned Depth, const Query &Q) {
     if (auto CS = ImmutableCallSite(V)) {
       if (CS.isReturnNonNull())
         return true;
-      if (CS.getIntrinsicID() == Intrinsic::ID::launder_invariant_group)
-        return isKnownNonZero(CS->getOperand(0), Depth + 1, Q);
+      if (const auto *RP = getArgumentAliasingToReturnedPointer(CS))
+        return isKnownNonZero(RP, Depth + 1, Q);
     }
   }
 
@@ -3385,6 +3385,22 @@ uint64_t llvm::GetStringLength(const Value *V, unsigned CharSize) {
   return Len == ~0ULL ? 1 : Len;
 }
 
+const Value *llvm::getArgumentAliasingToReturnedPointer(ImmutableCallSite CS) {
+  assert(CS &&
+         "getArgumentAliasingToReturnedPointer only works on nonnull CallSite");
+  if (const Value *RV = CS.getReturnedArgOperand())
+    return RV;
+  // This can be used only as a aliasing property.
+  if (isIntrinsicReturningPointerAliasingArgumentWithoutCapturing(CS))
+    return CS.getArgOperand(0);
+  return nullptr;
+}
+
+bool llvm::isIntrinsicReturningPointerAliasingArgumentWithoutCapturing(
+      ImmutableCallSite CS) {
+  return CS.getIntrinsicID() == Intrinsic::launder_invariant_group;
+}
+
 /// \p PN defines a loop-variant pointer to an object.  Check if the
 /// previous iteration of the loop was referring to the same object as \p PN.
 static bool isSameUnderlyingObjectInLoop(const PHINode *PN,
@@ -3430,11 +3446,19 @@ Value *llvm::GetUnderlyingObject(Value *V, const DataLayout &DL,
       // An alloca can't be further simplified.
       return V;
     } else {
-      if (auto CS = CallSite(V))
-        if (Value *RV = CS.getReturnedArgOperand()) {
-          V = RV;
+      if (auto CS = CallSite(V)) {
+        // Note: getArgumentAliasingToReturnedPointer keeps it in sync with
+        // CaptureTracking, which is needed for correctness.  This is because
+        // some intrinsics like launder.invariant.group returns pointers that
+        // are aliasing it's argument, which is known to CaptureTracking.
+        // If AliasAnalysis does not use the same information, it could assume
+        // that pointer returned from launder does not alias it's argument
+        // because launder could not return it if the pointer was not captured.
+        if (auto *RP = getArgumentAliasingToReturnedPointer(CS)) {
+          V = RP;
           continue;
         }
+      }
 
       // See if InstructionSimplify knows any relevant tricks.
       if (Instruction *I = dyn_cast<Instruction>(V))

llvm/test/Analysis/ValueTracking/known-nonnull-at.ll

@@ -98,3 +98,24 @@ exc:
   unreachable
 }
 
+declare i8* @returningPtr(i8* returned %p)
+
+define i1 @nonnullReturnTest(i8* nonnull %x) {
+; CHECK-LABEL: @nonnullReturnTest(
+; CHECK-NEXT:    %x2 = call i8* @returningPtr(i8* %x)
+; CHECK-NEXT:    ret i1 false
+  %x2 = call i8* @returningPtr(i8* %x)
+  %null_check = icmp eq i8* %x2, null
+  ret i1 %null_check
+}
+
+define i1 @unknownReturnTest(i8* %x) {
+; CHECK-LABEL: @unknownReturnTest(
+; CHECK-NEXT:    %x2 = call i8* @returningPtr(i8* %x)
+; CHECK-NEXT:    %null_check = icmp eq i8* %x2, null
+; CHECK-NEXT:    ret i1 %null_check
+  %x2 = call i8* @returningPtr(i8* %x)
+  %null_check = icmp eq i8* %x2, null
+  ret i1 %null_check
+}
+

llvm/test/Transforms/Inline/launder.invariant.group.ll

@@ -0,0 +1,31 @@
+; RUN: opt -S -inline < %s | FileCheck %s
+; RUN: opt -S -O3 < %s | FileCheck %s
+
+; This test checks if value returned from the launder is considered aliasing
+; with its argument.  Due to bug caused by handling launder in capture tracking
+; sometimes it would be considered noalias.
+
+%struct.A = type <{ i32 (...)**, i32, [4 x i8] }>
+
+; CHECK: define i32 @bar(%struct.A* noalias
+define i32 @bar(%struct.A* noalias) {
+; CHECK-NOT: noalias
+  %2 = bitcast %struct.A* %0 to i8*
+  %3 = call i8* @llvm.launder.invariant.group.p0i8(i8* %2)
+  %4 = getelementptr inbounds i8, i8* %3, i64 8
+  %5 = bitcast i8* %4 to i32*
+  store i32 42, i32* %5, align 8
+  %6 = getelementptr inbounds %struct.A, %struct.A* %0, i64 0, i32 1
+  %7 = load i32, i32* %6, align 8
+  ret i32 %7
+}
+
+; CHECK-LABEL: define i32 @foo(%struct.A* noalias
+define i32 @foo(%struct.A* noalias)  {
+  ; CHECK-NOT: call i32 @bar(
+  ; CHECK-NOT: noalias
+  %2 = tail call i32 @bar(%struct.A* %0)
+  ret i32 %2
+}
+
+declare i8* @llvm.launder.invariant.group.p0i8(i8*)