[PATCH] Specify the access behaviour of the memcpy, memmove and memset intrinsics
Arnaud Allard de Grandmaison
arnaud.adegm at gmail.com
Wed May 1 08:27:00 PDT 2013
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
More information about the llvm-commits
mailing list