[PATCH] D12310: Introducing llvm.invariant.group.barrier intrinsic & Global Opt handling

[Nick Lewycky via llvm-commits]

nlewycky added inline comments.

================
Comment at: include/llvm/IR/IRBuilder.h:1626-1627
@@ -1624,1 +1625,4 @@
 
+  /// \brief Create an invariant.group.barrier intrinsic call, that stops
+  /// optimizer to propagate equality using invariant.group metadata.
+  /// If Ptr type is different from i8*, it's casted to i8* before call
----------------
Suggested wording: Create a call to invariant.group.barrier which stops the optimizer from propagating equality with invariant.group metadata.

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:1414-1417
@@ -1413,2 +1413,6 @@
     }
+    case Intrinsic::invariant_group_barrier:
+      II->replaceAllUsesWith(II->getArgOperand(0));
+      II->eraseFromParent();
+      return true;
     }
----------------
Do we also discard the !invariant.group metadata on instructions?

The backend has pointers to the Value*'s, it would be bad if they used the !invariant.group markers but all the invariant.group.barrier calls had been removed.

================
Comment at: lib/Transforms/IPO/GlobalOpt.cpp:2507-2508
@@ -2506,1 +2506,4 @@
           continue;
+        } else if (II->getIntrinsicID() == Intrinsic::invariant_group_barrier) {
+          setVal(II, getVal(II->getOperand(0)));
+          DEBUG(dbgs() << "Passing through invariant.group.barrier intrinsic.\n");
----------------
Why is this safe? We have
  %p2 = call @invariant.group.barrier(%p1)
and we set that %p2 just plain *is* %p1. Then nothing happens until evaluation completes and we commit the changes, which means replacing %p2 with %p1. At the time, why won't that operation miscompile?

I can think of two possible reasons, one is that %p1 may be constrained (like, it may be a ConstantExpr) and the other is that we may know that there are no remaining loads of either %p1 or %p2 with !invariant.group on them.

In any event, please answer in the form of a comment in this code. :)


http://reviews.llvm.org/D12310