[PATCH] Specify the access behaviour of the memcpy, memmove and memset intrinsics

[Arnaud Allard de Grandmaison]

  I am fixing the patch right now and will resubmit it soon.


================
Comment at: include/llvm/IR/IntrinsicInst.h:135-144
@@ -134,4 +134,12 @@
+
     bool isVolatile() const {
-      return !getVolatileCst()->isZero();
+      switch (getIntrinsicID()) {
+        case Intrinsic::memcpy:
+        case Intrinsic::memmove:
+          return isDestVolatile() ||
+            !cast<ConstantInt>(const_cast<Value*>(getArgOperand(5)))->isZero();
+        default:
+          return isDestVolatile();
+      }
     }
 
----------------
Chandler Carruth wrote:
> I would just sink this into the specific intrinsic classes so that we don't have to get the intrinsic ID and switch over it here.
I aggree if we consider this method will stay --- but I think this method should just disappear as it has a dubious utility given the new interface. It is just kept with minimal changes for not breaking existing code and will be removed in a subsequent patch : all users of isVolatile have to take proper actions if the volatile attribute is set on src or dst.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:4472-4473
@@ -4471,3 +4471,4 @@
       Align = 1; // @llvm.memcpy defines 0 and 1 to both mean no alignment.
-    bool isVol = cast<ConstantInt>(I.getArgOperand(4))->getZExtValue();
+    bool isVol = cast<ConstantInt>(I.getArgOperand(4))->getZExtValue() ||
+                 cast<ConstantInt>(I.getArgOperand(5))->getZExtValue();
     DAG.setRoot(DAG.getMemcpy(getRoot(), dl, Op1, Op2, Op3, Align, isVol, false,
----------------
Chandler Carruth wrote:
> Update to use the new APIs?
I adapted to the surrounding style, which does not use interface methods. But using them is definitely clearer. And cleaner. Will fix the patch.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:4510-4512
@@ -4508,4 +4509,5 @@
       Align = 1; // @llvm.memmove defines 0 and 1 to both mean no alignment.
-    bool isVol = cast<ConstantInt>(I.getArgOperand(4))->getZExtValue();
+    bool isVol = cast<ConstantInt>(I.getArgOperand(4))->getZExtValue() ||
+                 cast<ConstantInt>(I.getArgOperand(5))->getZExtValue();
     DAG.setRoot(DAG.getMemmove(getRoot(), dl, Op1, Op2, Op3, Align, isVol,
                                MachinePointerInfo(I.getArgOperand(0)),
----------------
Chandler Carruth wrote:
> Here as well.
Will do.

================
Comment at: lib/Target/ARM/ARMFastISel.cpp:2292
@@ -2292,1 +2291,3 @@
+                             const char *IntrMemName = 0,
+                             unsigned numVolatileSpec = 0) {
   const CallInst *CI = cast<CallInst>(I);
----------------
Chandler Carruth wrote:
> This seems like a moderately horrible interface. I would much prefer to use the types to tell what to do than to count the volatile flags.
Will fix it.


http://llvm-reviews.chandlerc.com/D386