[CaptureTracking] Support atomicrmw and cmpxchg

[reames]

Summary

These atomic operations are conceptually both a load and store from the same location. As such, we can treat them as the most conservative of those two components which in practice, means we can treat them like stores. An cmpxchg or atomicrmw captures the values, but not the locations accessed.

Note: We can probably be more aggressive about the comparison value in an cmpxhg since to have it be in memory, it must already be captured, but I figured it was better to avoid that for the moment.

Note 2: It turns out that since we don't actually support cmpxchg of pointer type, writing a negative test is impossible.

[sanjoy]

lgtm

[jfb]

lgtm

Interesting: the current code can't see through an inttoptr + cmpxchg which captures the pointer? It assumes it's captured as soon as the inttoptr occurs?

[reames]

In D17400#356062, @jfb wrote:

Interesting: the current code can't see through an inttoptr + cmpxchg which captures the pointer? It assumes it's captured as soon as the inttoptr occurs?

Yes. Which is one of the many reasons we should just support cmpxchg on pointers directly. :)

[jfb]

In D17400#356077, @reames wrote:

In D17400#356062, @jfb wrote:

Interesting: the current code can't see through an inttoptr + cmpxchg which captures the pointer? It assumes it's captured as soon as the inttoptr occurs?

Yes. Which is one of the many reasons we should just support cmpxchg on pointers directly. :)

You've got my support :-)

[reames]

FYI, since we didn't seem to have one, I filed a bug for support pointer typed cmpxchgs: https://llvm.org/bugs/show_bug.cgi?id=26668.

lib/Analysis/CaptureTracking.cpp

case Instruction::VAArg:
       break;
     case Instruction::Store:
       if (V == I->getOperand(0))
         // Stored the pointer - conservatively assume it may be captured.
         if (Tracker->captured(U))
           return;
       // Storing to the pointee does not cause the pointer to be captured.
       break;
+    case Instruction::AtomicRMW:
+    case Instruction::AtomicCmpXchg:
+      // atomicrmw and cmpxchg conceptually include both a load and store from
+      // the same location.  As with a store, the location being accessed is
+      // not captured, but the value being stored is.  (For cmpxchg, we
+      // probably don't need to capture the original comparison value, but for
+      // the moment, let's be conservative.)
+      if (V != I->getOperand(0))
+        if (Tracker->captured(U))
+          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()) {

test/Transforms/FunctionAttrs/nocapture.ll

 
 ; CHECK: define void @test6_2(i8* %x6_2, i8* nocapture %y6_2, i8* %z6_2)
 define void @test6_2(i8* %x6_2, i8* %y6_2, i8* %z6_2) {
   call void (i8*, i8*, ...) @test6_1(i8* %x6_2, i8* %y6_2, i8* %z6_2)
   store i32* null, i32** @g
   ret void
 }
 
+; CHECK: define void @test_cmpxchg(i32* nocapture %p)
+define void @test_cmpxchg(i32* %p) {
+  cmpxchg i32* %p, i32 0, i32 1 acquire monotonic
+  ret void
+}
+
+; CHECK: define void @test_atomicrmw(i32* nocapture %p)
+define void @test_atomicrmw(i32* %p) {
+  atomicrmw add i32* %p, i32 1 seq_cst
+  ret void
+}