llvm.noalias - CaptureTracking needs to look through them

[hfinkel]

Summary

This is part of the series started by D9375, and teaches CaptureTracking to look through calls to llvm.noalias.

P.S. One reason why llvm.noalias is not itself marked with nocapture is because it requires this special handling anyway.

[reames]

LGTM w.r.t. code.

I would prefer the intrinsic definition be marked nocapture if it's legal to do so. There's too much chance of someone checking only an attribute somewhere. Better to be complete as we can.

Also, I don't believe the parameter on stripPointerCasts is still needed per discussion in another thread right?

[hfinkel]

Rebased.

[majnemer]

LGTM

lib/Analysis/CaptureTracking.cpp

255

auto *II

[hfinkel]

Rebased (and addressed reviewer feedback)

[jeroen.dobbelaere]

Maybe 'passthrough' should return a boolean, indicating that we need to stop or not. (see code comment)
The places where it is called could become something like: 'if (passthrough()) return;

lib/Analysis/CaptureTracking.cpp

244

Moving this piece of code with a 'return' into a lambda results in a behavior change: instead of directly breaking the outer while loop when the Threshold is reached, it will now keep doing work with whatever is on the worklist. I don't think this change in behavior was intended.

[hfinkel]

lib/Analysis/CaptureTracking.cpp

244

You're right, thanks!

[hfinkel]

Rebased.

lib/Analysis/CaptureTracking.cpp

 #include "llvm/Analysis/CFG.h"
 #include "llvm/Analysis/CaptureTracking.h"
 #include "llvm/Analysis/OrderedBasicBlock.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
+#include "llvm/IR/Intrinsics.h"
 
 using namespace llvm;
 
 CaptureTracker::~CaptureTracker() {}
 
 bool CaptureTracker::shouldExplore(const Use *U) { return true; }
 
 namespace {
for (const Use &U : V->uses()) {
     Worklist.push_back(&U);
   }
 
   while (!Worklist.empty()) {
     const Use *U = Worklist.pop_back_val();
     Instruction *I = cast<Instruction>(U->getUser());
     V = U->get();
 
+    auto Passthrough = [&]() {
+      // The original value is not captured via this if the new value isn't.
+      Count = 0;
+      for (Use &UU : I->uses()) {
+        // If there are lots of uses, conservatively say that the value
+        // is captured to avoid taking too much compile time.
+        if (Count++ >= Threshold)
+          return Tracker->tooManyUses();
+
+        if (Visited.insert(&UU).second)
+          if (Tracker->shouldExplore(&UU))
+            Worklist.push_back(&UU);
+      }
+    };
+
     switch (I->getOpcode()) {
     case Instruction::Call:
     case Instruction::Invoke: {
+      if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(I))
+        if (II->getIntrinsicID() == Intrinsic::noalias) {
+          Passthrough();
+          break;
+        }
+
       CallSite CS(I);
       // Not captured if the callee is readonly, doesn't return a copy through
       // its return value and doesn't unwind (a readonly function can leak bits
       // by throwing an exception or not depending on the input value).
       if (CS.onlyReadsMemory() && CS.doesNotThrow() && I->getType()->isVoidTy())
         break;
 
       // Volatile operations effectively capture the memory location that they
case Instruction::AtomicCmpXchg: {
           return;
       break;
     }
     case Instruction::BitCast:
     case Instruction::GetElementPtr:
     case Instruction::PHI:
     case Instruction::Select:
     case Instruction::AddrSpaceCast:
-      // The original value is not captured via this if the new value isn't.
-      Count = 0;
-      for (Use &UU : I->uses()) {
-        // If there are lots of uses, conservatively say that the value
-        // is captured to avoid taking too much compile time.
-        if (Count++ >= Threshold)
-          return Tracker->tooManyUses();
-
-        if (Visited.insert(&UU).second)
-          if (Tracker->shouldExplore(&UU))
-            Worklist.push_back(&UU);
-      }
+      Passthrough();
       break;
     case Instruction::ICmp: {
       // Don't count comparisons of a no-alias return value against null as
       // captures. This allows us to ignore comparisons of malloc results
       // with null, for example.
       if (ConstantPointerNull *CPN =
           dyn_cast<ConstantPointerNull>(I->getOperand(1)))
         if (CPN->getType()->getAddressSpace() == 0)

test/Transforms/FunctionAttrs/nocapture.ll

l:
   %tmp = bitcast i32* %x to i32*    ; <i32*> [#uses=2]
   %tmp2 = select i1 %b, i32* %tmp, i32* %y
   %val = load i32, i32* %tmp2    ; <i32> [#uses=1]
   store i32 0, i32* %tmp
   store i32* %y, i32** @g
   ret i32 %val
 }
 
+; CHECK: define i32 @nc1a(i32* %q, i32* nocapture %p, i1 %b)
+define i32 @nc1a(i32* %q, i32* %p, i1 %b) {
+e:
+        %pa = call i32* @llvm.noalias.p0i32(i32* %p, metadata !1)
+  br label %l
+l:
+  %x = phi i32* [ %pa, %e ]
+  %y = phi i32* [ %q, %e ]
+  %tmp = bitcast i32* %x to i32*    ; <i32*> [#uses=2]
+  %tmp2 = select i1 %b, i32* %tmp, i32* %y
+  %val = load i32, i32* %tmp2    ; <i32> [#uses=1]
+  store i32 0, i32* %tmp
+  store i32* %y, i32** @g
+  ret i32 %val
+}
+
 ; CHECK: define i32 @nc1_addrspace(i32* %q, i32 addrspace(1)* nocapture %p, i1 %b)
 define i32 @nc1_addrspace(i32* %q, i32 addrspace(1)* %p, i1 %b) {
 e:
   br label %l
 l:
   %x = phi i32 addrspace(1)* [ %p, %e ]
   %y = phi i32* [ %q, %e ]
   %tmp = addrspacecast i32 addrspace(1)* %x to i32*    ; <i32*> [#uses=2]

 
 ; CHECK: define void @test_volatile(i32* %x)
 define void @test_volatile(i32* %x) {
 entry:
   %gep = getelementptr i32, i32* %x, i64 1
   store volatile i32 0, i32* %gep, align 4
   ret void
 }
+
+declare i32* @llvm.noalias.p0i32(i32*, metadata) nounwind
+
+!0 = !{!0, !"some domain"}
+!1 = !{!1, !0, !"some scope"}
+