From d3b9f61c52a11516ea90de49e2db7823668b41c5 Mon Sep 17 00:00:00 2001 From: Simon Dardis Date: Fri, 3 Nov 2017 15:35:13 +0000 Subject: [PATCH] [mips] Match 'ins' and its' variants with C++ code Change the ISel matching of 'ins', 'dins[mu]' from tablegen code to C++ code. This resolves an issue where ISel would select 'dins' instead of 'dinsm' when the instructions size and position were individually in range but their sum was out of range according to the ISA specification. Reviewers: atanasyan Differential Revision: https://reviews.llvm.org/D39117 llvm-svn: 317331 --- llvm/lib/Target/Mips/MicroMips64r6InstrInfo.td | 7 ++-- llvm/lib/Target/Mips/MicroMipsInstrInfo.td | 2 +- llvm/lib/Target/Mips/Mips64InstrInfo.td | 6 +-- llvm/lib/Target/Mips/MipsInstrInfo.td | 9 ++-- llvm/lib/Target/Mips/MipsSEISelDAGToDAG.cpp | 58 ++++++++++++++++++++++++++ llvm/test/CodeGen/Mips/dins.ll | 14 ++++--- 6 files changed, 79 insertions(+), 17 deletions(-) diff --git a/llvm/lib/Target/Mips/MicroMips64r6InstrInfo.td b/llvm/lib/Target/Mips/MicroMips64r6InstrInfo.td index e0f4d83..4f705fe 100644 --- a/llvm/lib/Target/Mips/MicroMips64r6InstrInfo.td +++ b/llvm/lib/Target/Mips/MicroMips64r6InstrInfo.td @@ -162,12 +162,11 @@ class DCLZ_MM64R6_DESC { class DINSU_MM64R6_DESC : InsBase<"dinsu", GPR64Opnd, uimm5_plus32, uimm5_inssize_plus1, immZExt5Plus32, - immZExt5Plus1, MipsIns>; + immZExt5Plus1>; class DINSM_MM64R6_DESC : InsBase<"dinsm", GPR64Opnd, uimm5, uimm_range_2_64, - immZExt5, immZExtRange2To64, MipsIns>; + immZExt5, immZExtRange2To64>; class DINS_MM64R6_DESC : InsBase<"dins", GPR64Opnd, uimm5_report_uimm6, - uimm5_inssize_plus1, immZExt5, immZExt5Plus1, - MipsIns>; + uimm5_inssize_plus1, immZExt5, immZExt5Plus1>; class DMTC0_MM64R6_DESC : MTC0_MMR6_DESC_BASE<"dmtc0", COP0Opnd, GPR64Opnd, II_DMTC0>; class DMTC1_MM64R6_DESC : MTC1_MMR6_DESC_BASE<"dmtc1", FGR64Opnd, GPR64Opnd, diff --git a/llvm/lib/Target/Mips/MicroMipsInstrInfo.td b/llvm/lib/Target/Mips/MicroMipsInstrInfo.td index 1f869db..90399dd 100644 --- a/llvm/lib/Target/Mips/MicroMipsInstrInfo.td +++ b/llvm/lib/Target/Mips/MicroMipsInstrInfo.td @@ -884,7 +884,7 @@ let DecoderNamespace = "MicroMips", Predicates = [InMicroMips] in { def EXT_MM : MMRel, ExtBase<"ext", GPR32Opnd, uimm5, uimm5_plus1, immZExt5, immZExt5Plus1, MipsExt>, EXT_FM_MM<0x2c>; def INS_MM : MMRel, InsBase<"ins", GPR32Opnd, uimm5, uimm5_inssize_plus1, - immZExt5, immZExt5Plus1, MipsIns>, + immZExt5, immZExt5Plus1>, EXT_FM_MM<0x0c>; /// Jump Instructions diff --git a/llvm/lib/Target/Mips/Mips64InstrInfo.td b/llvm/lib/Target/Mips/Mips64InstrInfo.td index 04a050c..dbd47de 100644 --- a/llvm/lib/Target/Mips/Mips64InstrInfo.td +++ b/llvm/lib/Target/Mips/Mips64InstrInfo.td @@ -341,13 +341,13 @@ let AdditionalPredicates = [NotInMicroMips] in { // for dinsm and dinsu like binutils. let DecoderMethod = "DecodeDINS" in { def DINS : InsBase<"dins", GPR64Opnd, uimm6, uimm5_inssize_plus1, - immZExt5, immZExt5Plus1, MipsIns>, EXT_FM<7>, + immZExt5, immZExt5Plus1>, EXT_FM<7>, ISA_MIPS64R2; def DINSU : InsBase<"dinsu", GPR64Opnd, uimm5_plus32, uimm5_inssize_plus1, - immZExt5Plus32, immZExt5Plus1, MipsIns>, + immZExt5Plus32, immZExt5Plus1>, EXT_FM<6>, ISA_MIPS64R2; def DINSM : InsBase<"dinsm", GPR64Opnd, uimm5, uimm_range_2_64, - immZExt5, immZExtRange2To64, MipsIns>, + immZExt5, immZExtRange2To64>, EXT_FM<5>, ISA_MIPS64R2; } } diff --git a/llvm/lib/Target/Mips/MipsInstrInfo.td b/llvm/lib/Target/Mips/MipsInstrInfo.td index c4c3eb7..ac4980e 100644 --- a/llvm/lib/Target/Mips/MipsInstrInfo.td +++ b/llvm/lib/Target/Mips/MipsInstrInfo.td @@ -1726,12 +1726,13 @@ class ExtBase, ISA_MIPS32R2; +// 'ins' and its' 64 bit variants are matched by C++ code. class InsBase: + Operand SizeOpnd, PatFrag PosImm, PatFrag SizeImm>: InstSE<(outs RO:$rt), (ins RO:$rs, PosOpnd:$pos, SizeOpnd:$size, RO:$src), !strconcat(opstr, " $rt, $rs, $pos, $size"), - [(set RO:$rt, (Op RO:$rs, PosImm:$pos, SizeImm:$size, RO:$src))], + [(set RO:$rt, (null_frag RO:$rs, PosImm:$pos, SizeImm:$size, + RO:$src))], II_INS, FrmR, opstr>, ISA_MIPS32R2 { let Constraints = "$src = $rt"; } @@ -2236,7 +2237,7 @@ let AdditionalPredicates = [NotInMicroMips] in { EXT_FM<0>; def INS : MMRel, StdMMR6Rel, InsBase<"ins", GPR32Opnd, uimm5, uimm5_inssize_plus1, immZExt5, - immZExt5Plus1, MipsIns>, + immZExt5Plus1>, EXT_FM<4>; } /// Move Control Registers From/To CPU Registers diff --git a/llvm/lib/Target/Mips/MipsSEISelDAGToDAG.cpp b/llvm/lib/Target/Mips/MipsSEISelDAGToDAG.cpp index 283fcaa..3c6a7d7 100644 --- a/llvm/lib/Target/Mips/MipsSEISelDAGToDAG.cpp +++ b/llvm/lib/Target/Mips/MipsSEISelDAGToDAG.cpp @@ -905,6 +905,64 @@ bool MipsSEDAGToDAGISel::trySelect(SDNode *Node) { break; } + // Manually match MipsISD::Ins nodes to get the correct instruction. It has + // to be done in this fashion so that we respect the differences between + // dins and dinsm, as the difference is that the size operand has the range + // 0 < size <= 32 for dins while dinsm has the range 2 <= size <= 64 which + // means SelectionDAGISel would have to test all the operands at once to + // match the instruction. + case MipsISD::Ins: { + + // Sanity checking for the node operands. + if (Node->getValueType(0) != MVT::i32 && Node->getValueType(0) != MVT::i64) + return false; + + if (Node->getNumOperands() != 4) + return false; + + if (Node->getOperand(1)->getOpcode() != ISD::Constant || + Node->getOperand(2)->getOpcode() != ISD::Constant) + return false; + + MVT ResTy = Node->getSimpleValueType(0); + uint64_t Pos = Node->getConstantOperandVal(1); + uint64_t Size = Node->getConstantOperandVal(2); + + // Size has to be >0 for 'ins', 'dins' and 'dinsu'. + if (!Size) + return false; + + if (Pos + Size > 64) + return false; + + if (ResTy != MVT::i32 && ResTy != MVT::i64) + return false; + + unsigned Opcode = 0; + if (ResTy == MVT::i32) { + if (Pos + Size <= 32) + Opcode = Mips::INS; + } else { + if (Pos + Size <= 32) + Opcode = Mips::DINS; + else if (Pos < 32 && 1 < Size) + Opcode = Mips::DINSM; + else + Opcode = Mips::DINSU; + } + + if (Opcode) { + SDValue Ops[4] = { + Node->getOperand(0), CurDAG->getTargetConstant(Pos, DL, MVT::i32), + CurDAG->getTargetConstant(Size, DL, MVT::i32), Node->getOperand(3)}; + + ReplaceNode(Node, CurDAG->getMachineNode(Opcode, DL, ResTy, Ops)); + return true; + } + + return false; + } + case MipsISD::ThreadPointer: { EVT PtrVT = getTargetLowering()->getPointerTy(CurDAG->getDataLayout()); unsigned RdhwrOpc, DestReg; diff --git a/llvm/test/CodeGen/Mips/dins.ll b/llvm/test/CodeGen/Mips/dins.ll index 8a8b377..2f7138c 100644 --- a/llvm/test/CodeGen/Mips/dins.ll +++ b/llvm/test/CodeGen/Mips/dins.ll @@ -1,7 +1,11 @@ -; RUN: llc -O2 -march=mips64 -mcpu=mips64r2 -target-abi=n64 < %s -o - | FileCheck %s -check-prefix=MIPS64R2 -; RUN: llc -O2 -march=mips -mcpu=mips32r2 < %s -o - | FileCheck %s -check-prefix=MIPS32R2 -; RUN: llc -O2 -march=mips -mattr=mips16 < %s -o - | FileCheck %s -check-prefix=MIPS16 -; RUN: llc -O2 -march=mips64 -mcpu=mips64r2 -target-abi=n32 < %s -o - | FileCheck %s -check-prefix=MIPS64R2N32 +; RUN: llc -O2 -verify-machineinstrs -march=mips64 -mcpu=mips64r2 \ +; RUN: -target-abi=n64 < %s -o - | FileCheck %s -check-prefix=MIPS64R2 +; RUN: llc -O2 -verify-machineinstrs -march=mips -mcpu=mips32r2 < %s -o - \ +; RUN: | FileCheck %s -check-prefix=MIPS32R2 +; RUN: llc -O2 -verify-machineinstrs -march=mips -mattr=mips16 < %s -o - \ +; RUN: | FileCheck %s -check-prefix=MIPS16 +; RUN: llc -O2 -verify-machineinstrs -march=mips64 -mcpu=mips64r2 \ +; RUN: -target-abi=n32 < %s -o - | FileCheck %s -check-prefix=MIPS64R2N32 ; #include ; #include @@ -60,7 +64,7 @@ entry: ; MIPS64R2: daddiu $[[R0:[0-9]+]], $zero, 123 ; MIPS64R2: dinsm $[[R0:[0-9]+]], $[[R1:[0-9]+]], 27, 37 ; MIPS64R2: daddiu $[[R0:[0-9]+]], $zero, 4 -; MIPS64R2: dins $[[R0:[0-9]+]], $[[R1:[0-9]+]], 28, 6 +; MIPS64R2: dinsm $[[R0:[0-9]+]], $[[R1:[0-9]+]], 28, 6 ; MIPS64R2: daddiu $[[R0:[0-9]+]], $zero, 5 ; MIPS64R2: dinsu $[[R0:[0-9]+]], $[[R1:[0-9]+]], 50, 14 ; MIPS64R2: dsrl $[[R0:[0-9]+]], $[[R1:[0-9]+]], 50 -- 2.7.4