[PATCH] D21179: Add mrrc/mrrc2 intrinsics and update existing mcrr/mcrr2 intrinsics to accept a single uint64 type instead of 2 uint32 types

[Renato Golin via cfe-commits]

rengolin added inline comments.

================
Comment at: lib/CodeGen/CGBuiltin.cpp:3817
@@ +3816,3 @@
+
+    Value *Arg0 = EmitScalarExpr(E->getArg(0)); /* coproc */
+    Value *Arg1 = EmitScalarExpr(E->getArg(1)); /* opc1 */
----------------
Would be better to use the comments as names and avoid the comments...

It'd also make clear the casts below.

================
Comment at: lib/CodeGen/CGBuiltin.cpp:3824
@@ +3823,3 @@
+    Value *Arg2a = Builder.CreateTruncOrBitCast(Arg2, Int32Ty);
+    Value *Arg2b = Builder.CreateLShr(Arg2, C1);
+    Arg2b = Builder.CreateTruncOrBitCast(Arg2b, Int32Ty);
----------------
These variables could have better names...

================
Comment at: lib/CodeGen/CGBuiltin.cpp:3846
@@ +3845,3 @@
+    Value *Arg1 = EmitScalarExpr(E->getArg(1)); /* opc */
+    Value *Arg2 = EmitScalarExpr(E->getArg(2)); /* CRm */
+    Value *Val = Builder.CreateCall(F, {Arg0, Arg1, Arg2});
----------------
Same here

================
Comment at: test/CodeGen/builtins-arm.c:203
@@ -188,5 +202,3 @@
 
-void mcrr2(unsigned a, unsigned b) {
-  // CHECK: define void @mcrr2(i32 [[A:%.*]], i32 [[B:%.*]])
-  // CHECK: call void @llvm.arm.mcrr2(i32 15, i32 0, i32 [[A]], i32 [[B]], i32 0)
-  __builtin_arm_mcrr2(15, 0, a, b, 0);
+uint64_t mrrc2() {
+  // CHECK: define i64 @mrrc2()
----------------
I'm assuming this is a "soon to be documented" move, right?

It's really hard to review patches with these things changing all the time...


http://reviews.llvm.org/D21179