Add mrrc/mrrc2 co-processor intrinsics

[rs]

Summary

This patch adds codegen support to generate the mrrc/mrrc2 instruction from the intrinsic.

[rengolin]

lib/Target/ARM/Disassembler/ARMDisassembler.cpp

5285 ↗

(On Diff #60160)

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?

[rengolin]

lib/Target/ARM/ARMISelDAGToDAG.cpp

3303 ↗

(On Diff #60160)

Bad name, use "isThumbV8M".

[rs]

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

lib/Target/ARM/ARMISelDAGToDAG.cpp

3303 ↗

(On Diff #60160)

Actually I think I only need to do Subtarget->isThumb() not the rest.

lib/Target/ARM/Disassembler/ARMDisassembler.cpp

5285 ↗

(On Diff #60160)

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..

[rengolin]

lib/Target/ARM/Disassembler/ARMDisassembler.cpp

5285 ↗

(On Diff #60160)

If only MCRR2 and MRRC2 use this method, there's no point for the condition, right?

[rs]

lib/Target/ARM/Disassembler/ARMDisassembler.cpp

5285 ↗

(On Diff #60160)

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]

[rs]

lib/Target/ARM/Disassembler/ARMDisassembler.cpp

5285 ↗

(On Diff #60160)

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]

[rengolin]

Ok, LGTM with the { isThumb(), comment, function name } changes. Thanks!

lib/Target/ARM/Disassembler/ARMDisassembler.cpp

5268 ↗

(On Diff #60160)

Can you give this a more generic name? Now it decodes both mrrc2 and mcrr2...

5285 ↗

(On Diff #60160)

HA! Of course! Can you add a comment to that effect?

[rs]

Thanks for reviewing Renato, will make your suggested changes before committing. Can you also review the clang part of the patch at some point http://reviews.llvm.org/D21179 ?

Thanks,
Ranjeet

llvm/trunk/include/llvm/IR/IntrinsicsARM.td

 def int_arm_cdp : GCCBuiltin<"__builtin_arm_cdp">,
    Intrinsic<[], [llvm_i32_ty, llvm_i32_ty, llvm_i32_ty,
                   llvm_i32_ty, llvm_i32_ty, llvm_i32_ty], []>;
 def int_arm_cdp2 : GCCBuiltin<"__builtin_arm_cdp2">,
    Intrinsic<[], [llvm_i32_ty, llvm_i32_ty, llvm_i32_ty,
                   llvm_i32_ty, llvm_i32_ty, llvm_i32_ty], []>;
 
 // Move from two registers to coprocessor
-def int_arm_mcrr : GCCBuiltin<"__builtin_arm_mcrr">,
-   Intrinsic<[], [llvm_i32_ty, llvm_i32_ty, llvm_i32_ty,
+def int_arm_mcrr : Intrinsic<[], [llvm_i32_ty, llvm_i32_ty, llvm_i32_ty,
                                   llvm_i32_ty, llvm_i32_ty], []>;
-def int_arm_mcrr2 : GCCBuiltin<"__builtin_arm_mcrr2">,
-   Intrinsic<[], [llvm_i32_ty, llvm_i32_ty, llvm_i32_ty,
+def int_arm_mcrr2 : Intrinsic<[], [llvm_i32_ty, llvm_i32_ty, llvm_i32_ty,
+                                   llvm_i32_ty, llvm_i32_ty], []>;
+
+def int_arm_mrrc : Intrinsic<[llvm_i32_ty, llvm_i32_ty], [llvm_i32_ty,
+                              llvm_i32_ty, llvm_i32_ty], []>;
+def int_arm_mrrc2 : Intrinsic<[llvm_i32_ty, llvm_i32_ty], [llvm_i32_ty,
                                llvm_i32_ty, llvm_i32_ty], []>;
 
 //===----------------------------------------------------------------------===//
 // CRC32
 
 def int_arm_crc32b  : Intrinsic<[llvm_i32_ty], [llvm_i32_ty, llvm_i32_ty],
     [IntrNoMem]>;
 def int_arm_crc32cb : Intrinsic<[llvm_i32_ty], [llvm_i32_ty, llvm_i32_ty],
     [IntrNoMem]>;

llvm/trunk/lib/Target/ARM/ARMISelDAGToDAG.cpp

void ARMDAGToDAGISel::Select(SDNode *N) {
 
   case ISD::INTRINSIC_VOID:
   case ISD::INTRINSIC_W_CHAIN: {
     unsigned IntNo = cast<ConstantSDNode>(N->getOperand(1))->getZExtValue();
     switch (IntNo) {
     default:
       break;
 
+    case Intrinsic::arm_mrrc:
+    case Intrinsic::arm_mrrc2: {
+      SDLoc dl(N);
+      SDValue Chain = N->getOperand(0);
+      unsigned Opc;
+
+      if (Subtarget->isThumb())
+        Opc = (IntNo == Intrinsic::arm_mrrc ? ARM::t2MRRC : ARM::t2MRRC2);
+      else
+        Opc = (IntNo == Intrinsic::arm_mrrc ? ARM::MRRC : ARM::MRRC2);
+
+      SmallVector<SDValue, 5> Ops;
+      Ops.push_back(getI32Imm(cast<ConstantSDNode>(N->getOperand(2))->getZExtValue(), dl)); /* coproc */
+      Ops.push_back(getI32Imm(cast<ConstantSDNode>(N->getOperand(3))->getZExtValue(), dl)); /* opc */
+      Ops.push_back(getI32Imm(cast<ConstantSDNode>(N->getOperand(4))->getZExtValue(), dl)); /* CRm */
+      Ops.push_back(getAL(CurDAG, dl));
+      Ops.push_back(CurDAG->getRegister(0, MVT::i32));
+      Ops.push_back(Chain);
+
+      // Writes to two registers.
+      std::vector<EVT> RetType;
+      RetType.push_back(MVT::i32);
+      RetType.push_back(MVT::i32);
+      RetType.push_back(MVT::Other);
+
+      ReplaceNode(N, CurDAG->getMachineNode(Opc, dl, RetType, Ops));
+      return;
+    }
     case Intrinsic::arm_ldaexd:
     case Intrinsic::arm_ldrexd: {
       SDLoc dl(N);
       SDValue Chain = N->getOperand(0);
       SDValue MemAddr = N->getOperand(2);
       bool isThumb = Subtarget->isThumb() && Subtarget->hasV8MBaselineOps();
 
       bool IsAcquire = IntNo == Intrinsic::arm_ldaexd;

llvm/trunk/lib/Target/ARM/ARMInstrInfo.td

def MCRR : MovRRCopro<"mcrr", 0 /* from ARM core register to coprocessor */,
                       (outs), (ins p_imm:$cop, imm0_15:$opc1, GPRnopc:$Rt,
                       GPRnopc:$Rt2, c_imm:$CRm),
                       [(int_arm_mcrr imm:$cop, imm:$opc1, GPRnopc:$Rt,
                                      GPRnopc:$Rt2, imm:$CRm)]>;
 def MRRC : MovRRCopro<"mrrc", 1 /* from coprocessor to ARM core register */,
                       (outs GPRnopc:$Rt, GPRnopc:$Rt2),
                       (ins p_imm:$cop, imm0_15:$opc1, c_imm:$CRm), []>;
 
-class MovRRCopro2<string opc, bit direction, list<dag> pattern = []>
-  : ABXI<0b1100, (outs), (ins p_imm:$cop, imm0_15:$opc1,
-         GPRnopc:$Rt, GPRnopc:$Rt2, c_imm:$CRm), NoItinerary,
+class MovRRCopro2<string opc, bit direction, dag oops, dag iops,
+                  list<dag> pattern = []>
+  : ABXI<0b1100, oops, iops, NoItinerary,
          !strconcat(opc, "\t$cop, $opc1, $Rt, $Rt2, $CRm"), pattern>,
     Requires<[PreV8]> {
   let Inst{31-28} = 0b1111;
   let Inst{23-21} = 0b010;
   let Inst{20} = direction;
 
   bits<4> Rt;
   bits<4> Rt2;
   bits<4> cop;
   bits<4> opc1;
   bits<4> CRm;
 
   let Inst{15-12} = Rt;
   let Inst{19-16} = Rt2;
   let Inst{11-8}  = cop;
   let Inst{7-4}   = opc1;
   let Inst{3-0}   = CRm;
 
-  let DecoderMethod = "DecodeMRRC2";
+  let DecoderMethod = "DecoderForMRRC2AndMCRR2";
 }
 
 def MCRR2 : MovRRCopro2<"mcrr2", 0 /* from ARM core register to coprocessor */,
+                        (outs), (ins p_imm:$cop, imm0_15:$opc1, GPRnopc:$Rt,
+                        GPRnopc:$Rt2, c_imm:$CRm),
                         [(int_arm_mcrr2 imm:$cop, imm:$opc1, GPRnopc:$Rt,
                                         GPRnopc:$Rt2, imm:$CRm)]>;
-def MRRC2 : MovRRCopro2<"mrrc2", 1 /* from coprocessor to ARM core register */>;
+
+def MRRC2 : MovRRCopro2<"mrrc2", 1 /* from coprocessor to ARM core register */,
+                       (outs GPRnopc:$Rt, GPRnopc:$Rt2),
+                       (ins p_imm:$cop, imm0_15:$opc1, c_imm:$CRm), []>;
 
 //===----------------------------------------------------------------------===//
 // Move between special register and ARM core register
 //
 
 // Move to ARM core register from Special Register
 def MRS : ABI<0b0001, (outs GPRnopc:$Rd), (ins), NoItinerary,
               "mrs", "\t$Rd, apsr", []> {

llvm/trunk/lib/Target/ARM/Disassembler/ARMDisassembler.cpp

static DecodeStatus DecodeT2Adr(MCInst &Inst, unsigned Val,
                                 uint64_t Address, const void *Decoder);
 static DecodeStatus DecodeT2LdStPre(MCInst &Inst, unsigned Val,
                                 uint64_t Address, const void *Decoder);
 static DecodeStatus DecodeT2ShifterImmOperand(MCInst &Inst, unsigned Val,
                                 uint64_t Address, const void *Decoder);
 
 static DecodeStatus DecodeLDR(MCInst &Inst, unsigned Val,
                                 uint64_t Address, const void *Decoder);
-static DecodeStatus DecodeMRRC2(llvm::MCInst &Inst, unsigned Val,
+static DecodeStatus DecoderForMRRC2AndMCRR2(llvm::MCInst &Inst, unsigned Val,
                                             uint64_t Address, const void *Decoder);
 #include "ARMGenDisassemblerTables.inc"
 
 static MCDisassembler *createARMDisassembler(const Target &T,
                                              const MCSubtargetInfo &STI,
                                              MCContext &Ctx) {
   return new ARMDisassembler(STI, Ctx);
 }
 
static DecodeStatus DecodeLDR(MCInst &Inst, unsigned Val,
   if (!Check(S, DecodePostIdxReg(Inst, Rm, Address, Decoder)))
     return MCDisassembler::Fail;
   if (!Check(S, DecodePredicateOperand(Inst, Cond, Address, Decoder)))
     return MCDisassembler::Fail;
 
   return S;
 }
 
-static DecodeStatus DecodeMRRC2(llvm::MCInst &Inst, unsigned Val,
+static DecodeStatus DecoderForMRRC2AndMCRR2(llvm::MCInst &Inst, unsigned Val,
                                             uint64_t Address, const void *Decoder) {
 
   DecodeStatus S = MCDisassembler::Success;
 
   unsigned CRm = fieldFromInstruction(Val, 0, 4);
   unsigned opc1 = fieldFromInstruction(Val, 4, 4);
   unsigned cop = fieldFromInstruction(Val, 8, 4);
   unsigned Rt = fieldFromInstruction(Val, 12, 4);
   unsigned Rt2 = fieldFromInstruction(Val, 16, 4);
 
   if ((cop & ~0x1) == 0xa)
     return MCDisassembler::Fail;
 
   if (Rt == Rt2)
     S = MCDisassembler::SoftFail;
 
+  // We have to check if the instruction is MRRC2
+  // or MCRR2 when constructing the operands for
+  // Inst. Reason is because MRRC2 stores to two
+  // registers so it's tablegen desc has has two
+  // outputs whereas MCRR doesn't store to any
+  // registers so all of it's operands are listed
+  // as inputs, therefore the operand order for
+  // MRRC2 needs to be [Rt, Rt2, cop, opc1, CRm]
+  // and MCRR2 operand order is [cop, opc1, Rt, Rt2, CRm]
+
+  if (Inst.getOpcode() == ARM::MRRC2) {
+    if (!Check(S, DecodeGPRnopcRegisterClass(Inst, Rt, Address, Decoder)))
+      return MCDisassembler::Fail;
+    if (!Check(S, DecodeGPRnopcRegisterClass(Inst, Rt2, Address, Decoder)))
+      return MCDisassembler::Fail;
+  }
   Inst.addOperand(MCOperand::createImm(cop));
   Inst.addOperand(MCOperand::createImm(opc1));
+  if (Inst.getOpcode() == ARM::MCRR2) {
     if (!Check(S, DecodeGPRnopcRegisterClass(Inst, Rt, Address, Decoder)))
       return MCDisassembler::Fail;
     if (!Check(S, DecodeGPRnopcRegisterClass(Inst, Rt2, Address, Decoder)))
       return MCDisassembler::Fail;
+  }
   Inst.addOperand(MCOperand::createImm(CRm));
 
   return S;
 }
 

llvm/trunk/test/CodeGen/ARM/intrinsics-coprocessor.ll

entry:
   ; CHECK: stc p7, c3, [r{{[0-9]+}}]
   tail call void @llvm.arm.stc(i32 7, i32 3, i8* %i) nounwind
   ; CHECK: stcl p7, c3, [r{{[0-9]+}}]
   tail call void @llvm.arm.stcl(i32 7, i32 3, i8* %i) nounwind
   ; CHECK: stc2 p7, c3, [r{{[0-9]+}}]
   tail call void @llvm.arm.stc2(i32 7, i32 3, i8* %i) nounwind
   ; CHECK: stc2l p7, c3, [r{{[0-9]+}}]
   tail call void @llvm.arm.stc2l(i32 7, i32 3, i8* %i) nounwind
+  ; CHECK: mrrc p1, #2, r{{[0-9]+}}, r{{[0-9]+}}, c3
+  %2 = tail call { i32, i32 } @llvm.arm.mrrc(i32 1, i32 2, i32 3) nounwind
+  ; CHECK: mrrc2 p1, #2, r{{[0-9]+}}, r{{[0-9]+}}, c3
+  %3 = tail call { i32, i32 } @llvm.arm.mrrc2(i32 1, i32 2, i32 3) nounwind
   ret void
 }
 
 declare void @llvm.arm.ldc(i32, i32, i8*) nounwind
 
 declare void @llvm.arm.ldcl(i32, i32, i8*) nounwind
 
 declare void @llvm.arm.ldc2(i32, i32, i8*) nounwind

 
 declare void @llvm.arm.mcr2(i32, i32, i32, i32, i32, i32) nounwind
 
 declare i32 @llvm.arm.mrc2(i32, i32, i32, i32, i32) nounwind
 
 declare void @llvm.arm.mcr(i32, i32, i32, i32, i32, i32) nounwind
 
 declare i32 @llvm.arm.mrc(i32, i32, i32, i32, i32) nounwind
+
+declare { i32, i32 } @llvm.arm.mrrc(i32, i32, i32) nounwind
+
+declare { i32, i32 } @llvm.arm.mrrc2(i32, i32, i32) nounwind