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

[Ranjeet Singh via llvm-commits]

rs added inline comments.

================
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)))
----------------
rs wrote:
> rengolin wrote:
> > rs wrote:
> > > 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.. 
> > If only MCRR2 and MRRC2 use this method, there's no point for the condition, right?
> The conditional checks are for controlling the order in which the operands are added to Inst. For MRRC2, the order should be [Rt, Rt2, coproc, opc1, CRm] for MCRR2 it should be [coproc, opc1, Rt, Rt2, CRm]
Just expanding on my previous answer. Previously the order in which variable Inst was constructed would be the same for both MRRC2 and MCRR2 because the tablgen description of the outputs and inputs was "...(outs), (ins p_imm:$cop, imm0_15:$opc1, GPRnopc:$Rt, GPRnopc:$Rt2, c_imm:$CRm...>". This  tablegen is correct for MCRR2 but not MRRC2, MRRC2 writes to 2 registers so the tablegen description should be "...(outs GPRnopc:$Rt, GPRnopc:$Rt2), (ins p_imm:$cop, imm0_15:$opc1, c_imm:$CRm)...". This is why the operands in Inst for MRRC2 need to be [Rt, Rt2, coproc, opc1, CRm]


http://reviews.llvm.org/D21178