[PATCH] D21178: Add mrrc/mrrc2 co-processor intrinsics

[Ranjeet Singh via llvm-commits]

rs added a comment.

Thanks for the review Renato. I replied back to your comments below.


================
Comment at: lib/Target/ARM/ARMISelDAGToDAG.cpp:3303
@@ +3302,3 @@
+      SDValue Chain = N->getOperand(0);
+      bool isThumb = Subtarget->isThumb() && Subtarget->hasV8MBaselineOps();
+      unsigned Opc;
----------------
rengolin wrote:
> Bad name, use "isThumbV8M".
Actually I think I only need to do Subtarget->isThumb() not the rest.

================
Comment at: lib/Target/ARM/Disassembler/ARMDisassembler.cpp:5285
@@ -5284,1 +5284,3 @@
 
+  if (Inst.getOpcode() == ARM::MRRC2) {
+    if (!Check(S, DecodeGPRnopcRegisterClass(Inst, Rt, Address, Decoder)))
----------------
rengolin wrote:
> This looks like a different change, for another patch.
> 
> Also, why only validate for MRRC and MCRR? Were the checks wrong for the rest?
> 
> If so, why not just have one check if MRRC || MCRR?
>This looks like a different change, for another patch.

Decoder tests were failing because I changed the tablegen definitions for 'MCRR2' and 'MRRC2' to define the inputs and ouputs in 'lib/Target/ARM/ARMInstrInfo.td'.

>Also, why only validate for MRRC and MCRR? Were the checks wrong for the rest?

This decoder method is only used by definitions instantiated from the tablegen class MovRRCopro2, there are only two definitions that use this class, they are MCRR2 and MRRC2.. 


http://reviews.llvm.org/D21178