[mips] Range check uimm20 and fixed a bug this revealed.

Summary: The bug was that dextu's operand 3 would print 0-31 instead of 32-63 when printing assembly. This came up when replacing MipsInstPrinter::printUnsignedImm() with a version that could handle arbitrary bit widths. MipsAsmPrinter::printUnsignedImm*() don't seem to be used so they have been removed. Reviewers: vkalintiris Subscribers: dsanders, llvm-commits Differential Revision: http://reviews.llvm.org/D15521 llvm-svn: 262231
llvm · Feb 29, 2016 · 03a8d2f · 03a8d2f
1 parent 29620ac
commit 03a8d2f
Show file tree

Hide file tree

Showing 28 changed files with 109 additions and 88 deletions.
diff --git a/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp b/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
@@ -3776,6 +3776,9 @@ bool MipsAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
   case Match_UImm16_Relaxed:
     return Error(RefineErrorLoc(IDLoc, Operands, ErrorInfo),
                  "expected 16-bit unsigned immediate");
+  case Match_UImm20_0:
+    return Error(RefineErrorLoc(IDLoc, Operands, ErrorInfo),
+                 "expected 20-bit unsigned immediate");
   }
 
   llvm_unreachable("Implement any new match types added!");

diff --git a/llvm/lib/Target/Mips/InstPrinter/MipsInstPrinter.cpp b/llvm/lib/Target/Mips/InstPrinter/MipsInstPrinter.cpp
@@ -203,22 +203,19 @@ void MipsInstPrinter::printOperand(const MCInst *MI, unsigned OpNo,
   printExpr(Op.getExpr(), &MAI, O);
 }
 
-void MipsInstPrinter::printUnsignedImm(const MCInst *MI, int opNum,
-                                       raw_ostream &O) {
+template <unsigned Bits, unsigned Offset>
+void MipsInstPrinter::printUImm(const MCInst *MI, int opNum, raw_ostream &O) {
   const MCOperand &MO = MI->getOperand(opNum);
-  if (MO.isImm())
-    O << (unsigned short int)MO.getImm();
-  else
-    printOperand(MI, opNum, O);
-}
+  if (MO.isImm()) {
+    uint64_t Imm = MO.getImm();
+    Imm -= Offset;
+    Imm &= (1 << Bits) - 1;
+    Imm += Offset;
+    O << Imm;
+    return;
+  }
 
-void MipsInstPrinter::printUnsignedImm8(const MCInst *MI, int opNum,
-                                        raw_ostream &O) {
-  const MCOperand &MO = MI->getOperand(opNum);
-  if (MO.isImm())
-    O << (unsigned short int)(unsigned char)MO.getImm();
-  else
-    printOperand(MI, opNum, O);
+  printOperand(MI, opNum, O);
 }
 
 void MipsInstPrinter::
@@ -343,7 +340,7 @@ void MipsInstPrinter::printSaveRestore(const MCInst *MI, raw_ostream &O) {
     if (MI->getOperand(i).isReg())
       printRegName(O, MI->getOperand(i).getReg());
     else
-      printUnsignedImm(MI, i, O);
+      printUImm<16>(MI, i, O);
   }
 }
 

diff --git a/llvm/lib/Target/Mips/InstPrinter/MipsInstPrinter.h b/llvm/lib/Target/Mips/InstPrinter/MipsInstPrinter.h
@@ -93,8 +93,8 @@ class MipsInstPrinter : public MCInstPrinter {
 
 private:
   void printOperand(const MCInst *MI, unsigned OpNo, raw_ostream &O);
-  void printUnsignedImm(const MCInst *MI, int opNum, raw_ostream &O);
-  void printUnsignedImm8(const MCInst *MI, int opNum, raw_ostream &O);
+  template <unsigned Bits, unsigned Offset = 0>
+  void printUImm(const MCInst *MI, int opNum, raw_ostream &O);
   void printMemOperand(const MCInst *MI, int opNum, raw_ostream &O);
   void printMemOperandEA(const MCInst *MI, int opNum, raw_ostream &O);
   void printFCCOperand(const MCInst *MI, int opNum, raw_ostream &O);

diff --git a/llvm/lib/Target/Mips/MicroMipsInstrInfo.td b/llvm/lib/Target/Mips/MicroMipsInstrInfo.td
@@ -891,7 +891,7 @@ let DecoderNamespace = "MicroMips", Predicates = [InMicroMips] in {
   /// Control Instructions
   def SYNC_MM    : MMRel, SYNC_FT<"sync">, SYNC_FM_MM;
   def BREAK_MM   : MMRel, BRK_FT<"break">, BRK_FM_MM;
-  def SYSCALL_MM : MMRel, SYS_FT<"syscall">, SYS_FM_MM;
+  def SYSCALL_MM : MMRel, SYS_FT<"syscall", uimm10>, SYS_FM_MM;
   def WAIT_MM    : WaitMM<"wait">, WAIT_FM_MM;
   def ERET_MM    : MMRel, ER_FT<"eret">, ER_FM_MM<0x3cd>;
   def DERET_MM   : MMRel, ER_FT<"deret">, ER_FM_MM<0x38d>;
@@ -944,7 +944,7 @@ let DecoderNamespace = "MicroMips", Predicates = [InMicroMips] in {
   def TLBWI_MM : MMRel, TLB<"tlbwi">, COP0_TLB_FM_MM<0x8d>;
   def TLBWR_MM : MMRel, TLB<"tlbwr">, COP0_TLB_FM_MM<0xcd>;
 
-  def SDBBP_MM : MMRel, SYS_FT<"sdbbp">, SDBBP_FM_MM;
+  def SDBBP_MM : MMRel, SYS_FT<"sdbbp", uimm10>, SDBBP_FM_MM;
 
   def PREFX_MM : PrefetchIndexed<"prefx">, POOL32F_PREFX_FM_MM<0x15, 0x1A0>;
 }

diff --git a/llvm/lib/Target/Mips/MipsAsmPrinter.cpp b/llvm/lib/Target/Mips/MipsAsmPrinter.cpp
@@ -620,24 +620,6 @@ void MipsAsmPrinter::printOperand(const MachineInstr *MI, int opNum,
   if (closeP) O << ")";
 }
 
-void MipsAsmPrinter::printUnsignedImm(const MachineInstr *MI, int opNum,
-                                      raw_ostream &O) {
-  const MachineOperand &MO = MI->getOperand(opNum);
-  if (MO.isImm())
-    O << (unsigned short int)MO.getImm();
-  else
-    printOperand(MI, opNum, O);
-}
-
-void MipsAsmPrinter::printUnsignedImm8(const MachineInstr *MI, int opNum,
-                                       raw_ostream &O) {
-  const MachineOperand &MO = MI->getOperand(opNum);
-  if (MO.isImm())
-    O << (unsigned short int)(unsigned char)MO.getImm();
-  else
-    printOperand(MI, opNum, O);
-}
-
 void MipsAsmPrinter::
 printMemOperand(const MachineInstr *MI, int opNum, raw_ostream &O) {
   // Load/Store memory operands -- imm($reg)

diff --git a/llvm/lib/Target/Mips/MipsAsmPrinter.h b/llvm/lib/Target/Mips/MipsAsmPrinter.h
@@ -134,8 +134,6 @@ class LLVM_LIBRARY_VISIBILITY MipsAsmPrinter : public AsmPrinter {
                              unsigned AsmVariant, const char *ExtraCode,
                              raw_ostream &O) override;
   void printOperand(const MachineInstr *MI, int opNum, raw_ostream &O);
-  void printUnsignedImm(const MachineInstr *MI, int opNum, raw_ostream &O);
-  void printUnsignedImm8(const MachineInstr *MI, int opNum, raw_ostream &O);
   void printMemOperand(const MachineInstr *MI, int opNum, raw_ostream &O);
   void printMemOperandEA(const MachineInstr *MI, int opNum, raw_ostream &O);
   void printFCCOperand(const MachineInstr *MI, int opNum, raw_ostream &O,

diff --git a/llvm/lib/Target/Mips/MipsInstrInfo.td b/llvm/lib/Target/Mips/MipsInstrInfo.td
@@ -422,8 +422,10 @@ class UImmAsmOperandClass<int Bits, list<AsmOperandClass> Supers = []>
   let DiagnosticType = "UImm" # Bits;
 }
 
+def ConstantUImm20AsmOperandClass
+    : ConstantUImmAsmOperandClass<20, []>;
 def UImm16RelaxedAsmOperandClass
-    : UImmAsmOperandClass<16, []> {
+    : UImmAsmOperandClass<16, [ConstantUImm20AsmOperandClass]> {
   let Name = "UImm16_Relaxed";
   let PredicateMethod = "isAnyImm<16>";
   let DiagnosticType = "UImm16_Relaxed";
@@ -541,70 +543,67 @@ def simm18_lsl3 : Operand<i32> {
 def simm20      : Operand<i32>;
 def simm32      : Operand<i32>;
 
-def uimm20      : Operand<i32> {
-}
-
 def simm16_64   : Operand<i64> {
   let DecoderMethod = "DecodeSimm16";
 }
 
 // Zero
 def uimmz       : Operand<i32> {
-  let PrintMethod = "printUnsignedImm";
+  let PrintMethod = "printUImm<0>";
   let ParserMatchClass = ConstantImmzAsmOperandClass;
 }
 
 // size operand of ins instruction
 def uimm_range_2_64 : Operand<i32> {
-  let PrintMethod = "printUnsignedImm";
+  let PrintMethod = "printUImm<6, 2>";
   let EncoderMethod = "getSizeInsEncoding";
   let DecoderMethod = "DecodeInsSize";
   let ParserMatchClass = ConstantUImm5_Range2_64AsmOperandClass;
 }
 
 // Unsigned Operands
-foreach I = {1, 2, 3, 4, 5, 6, 7, 8, 10} in
+foreach I = {1, 2, 3, 4, 5, 6, 7, 8, 10, 20} in
   def uimm # I : Operand<i32> {
-    let PrintMethod = "printUnsignedImm";
+    let PrintMethod = "printUImm<" # I # ">";
     let ParserMatchClass =
         !cast<AsmOperandClass>("ConstantUImm" # I # "AsmOperandClass");
   }
 
 def uimm2_plus1 : Operand<i32> {
-  let PrintMethod = "printUnsignedImm";
+  let PrintMethod = "printUImm<2, 1>";
   let EncoderMethod = "getUImmWithOffsetEncoding<2, 1>";
   let DecoderMethod = "DecodeUImmWithOffset<2, 1>";
   let ParserMatchClass = ConstantUImm2Plus1AsmOperandClass;
 }
 
 def uimm5_plus1 : Operand<i32> {
-  let PrintMethod = "printUnsignedImm";
+  let PrintMethod = "printUImm<5, 1>";
   let EncoderMethod = "getUImmWithOffsetEncoding<5, 1>";
   let DecoderMethod = "DecodeUImmWithOffset<5, 1>";
   let ParserMatchClass = ConstantUImm5Plus1AsmOperandClass;
 }
 
 def uimm5_plus32 : Operand<i32> {
-  let PrintMethod = "printUnsignedImm";
+  let PrintMethod = "printUImm<5, 32>";
   let ParserMatchClass = ConstantUImm5Plus32AsmOperandClass;
 }
 
 def uimm5_plus33 : Operand<i32> {
-  let PrintMethod = "printUnsignedImm";
+  let PrintMethod = "printUImm<5, 33>";
   let EncoderMethod = "getUImmWithOffsetEncoding<5, 1>";
   let DecoderMethod = "DecodeUImmWithOffset<5, 1>";
   let ParserMatchClass = ConstantUImm5Plus33AsmOperandClass;
 }
 
 def uimm5_inssize_plus1 : Operand<i32> {
-  let PrintMethod = "printUnsignedImm";
+  let PrintMethod = "printUImm<6>";
   let ParserMatchClass = ConstantUImm5Plus1AsmOperandClass;
   let EncoderMethod = "getSizeInsEncoding";
   let DecoderMethod = "DecodeInsSize";
 }
 
 def uimm5_plus32_normalize : Operand<i32> {
-  let PrintMethod = "printUnsignedImm";
+  let PrintMethod = "printUImm<5>";
   let ParserMatchClass = ConstantUImm5Plus32NormalizeAsmOperandClass;
 }
 
@@ -615,55 +614,55 @@ def uimm5_lsl2 : Operand<OtherVT> {
 }
 
 def uimm5_plus32_normalize_64 : Operand<i64> {
-  let PrintMethod = "printUnsignedImm";
+  let PrintMethod = "printUImm<5>";
   let ParserMatchClass = ConstantUImm5Plus32NormalizeAsmOperandClass;
 }
 
 foreach I = {16} in
   def uimm # I : Operand<i32> {
-    let PrintMethod = "printUnsignedImm";
+    let PrintMethod = "printUImm<16>";
     let ParserMatchClass =
         !cast<AsmOperandClass>("UImm" # I # "AsmOperandClass");
   }
 
 // Like uimm16_64 but coerces simm16 to uimm16.
 def uimm16_relaxed : Operand<i32> {
-  let PrintMethod = "printUnsignedImm";
+  let PrintMethod = "printUImm<16>";
   let ParserMatchClass =
       !cast<AsmOperandClass>("UImm16RelaxedAsmOperandClass");
 }
 
 foreach I = {5} in
   def uimm # I # _64 : Operand<i64> {
-    let PrintMethod = "printUnsignedImm";
+    let PrintMethod = "printUImm<5>";
     let ParserMatchClass =
         !cast<AsmOperandClass>("ConstantUImm" # I # "AsmOperandClass");
   }
 
 def uimm16_64 : Operand<i64> {
-  let PrintMethod = "printUnsignedImm";
+  let PrintMethod = "printUImm<16>";
   let ParserMatchClass =
       !cast<AsmOperandClass>("UImm16AsmOperandClass");
 }
 
 // Like uimm16_64 but coerces simm16 to uimm16.
 def uimm16_64_relaxed : Operand<i64> {
-  let PrintMethod = "printUnsignedImm";
+  let PrintMethod = "printUImm<16>";
   let ParserMatchClass =
       !cast<AsmOperandClass>("UImm16RelaxedAsmOperandClass");
 }
 
 // Like uimm5 but reports a less confusing error for 32-63 when
 // an instruction alias permits that.
 def uimm5_report_uimm6 : Operand<i32> {
-  let PrintMethod = "printUnsignedImm";
+  let PrintMethod = "printUImm<5>";
   let ParserMatchClass = ConstantUImm5ReportUImm6AsmOperandClass;
 }
 
 // Like uimm5_64 but reports a less confusing error for 32-63 when
 // an instruction alias permits that.
 def uimm5_64_report_uimm6 : Operand<i64> {
-  let PrintMethod = "printUnsignedImm";
+  let PrintMethod = "printUImm<5>";
   let ParserMatchClass = ConstantUImm5ReportUImm6AsmOperandClass;
 }
 
@@ -1135,8 +1134,8 @@ class BAL_BR_Pseudo<Instruction RealInst> :
 }
 
 // Syscall
-class SYS_FT<string opstr> :
-  InstSE<(outs), (ins uimm20:$code_),
+class SYS_FT<string opstr, Operand ImmOp> :
+  InstSE<(outs), (ins ImmOp:$code_),
          !strconcat(opstr, "\t$code_"), [], NoItinerary, FrmI, opstr>;
 // Break
 class BRK_FT<string opstr> :
@@ -1332,11 +1331,11 @@ class SCBase<string opstr, RegisterOperand RO> :
 }
 
 class MFC3OP<string asmstr, RegisterOperand RO, RegisterOperand RD> :
-  InstSE<(outs RO:$rt), (ins RD:$rd, uimm16:$sel),
+  InstSE<(outs RO:$rt), (ins RD:$rd, uimm3:$sel),
          !strconcat(asmstr, "\t$rt, $rd, $sel"), [], NoItinerary, FrmFR>;
 
 class MTC3OP<string asmstr, RegisterOperand RO, RegisterOperand RD> :
-  InstSE<(outs RO:$rd), (ins RD:$rt, uimm16:$sel),
+  InstSE<(outs RO:$rd), (ins RD:$rt, uimm3:$sel),
          !strconcat(asmstr, "\t$rt, $rd, $sel"), [], NoItinerary, FrmFR>;
 
 class TrapBase<Instruction RealInst>
@@ -1571,10 +1570,12 @@ def TNEI : MMRel, TEQI_FT<"tnei", GPR32Opnd>, TEQI_FM<0xe>,
 
 let AdditionalPredicates = [NotInMicroMips] in {
 def BREAK : MMRel, StdMMR6Rel, BRK_FT<"break">, BRK_FM<0xd>;
+def SYSCALL : MMRel, SYS_FT<"syscall", uimm20>, SYS_FM<0xc>;
 }
-def SYSCALL : MMRel, SYS_FT<"syscall">, SYS_FM<0xc>;
 def TRAP : TrapBase<BREAK>;
-def SDBBP : MMRel, SYS_FT<"sdbbp">, SDBBP_FM, ISA_MIPS32_NOT_32R6_64R6;
+let AdditionalPredicates = [NotInMicroMips] in {
+def SDBBP : MMRel, SYS_FT<"sdbbp", uimm20>, SDBBP_FM, ISA_MIPS32_NOT_32R6_64R6;
+}
 
 let AdditionalPredicates = [NotInMicroMips] in {
   def ERET : MMRel, ER_FT<"eret">, ER_FM<0x18, 0x0>, INSN_MIPS3_32;

diff --git a/llvm/lib/Target/Mips/MipsMSAInstrInfo.td b/llvm/lib/Target/Mips/MipsMSAInstrInfo.td
@@ -71,41 +71,41 @@ def immZExt6Ptr : ImmLeaf<iPTR, [{return isUInt<6>(Imm);}]>;
 // Operands
 
 def uimm4_ptr : Operand<iPTR> {
-  let PrintMethod = "printUnsignedImm8";
+  let PrintMethod = "printUImm<8>";
 }
 
 def uimm6_ptr : Operand<iPTR> {
-  let PrintMethod = "printUnsignedImm8";
+  let PrintMethod = "printUImm<8>";
 }
 
 def simm5 : Operand<i32>;
 
 def vsplat_uimm1 : Operand<vAny> {
-  let PrintMethod = "printUnsignedImm8";
+  let PrintMethod = "printUImm<8>";
 }
 
 def vsplat_uimm2 : Operand<vAny> {
-  let PrintMethod = "printUnsignedImm8";
+  let PrintMethod = "printUImm<8>";
 }
 
 def vsplat_uimm3 : Operand<vAny> {
-  let PrintMethod = "printUnsignedImm8";
+  let PrintMethod = "printUImm<8>";
 }
 
 def vsplat_uimm4 : Operand<vAny> {
-  let PrintMethod = "printUnsignedImm8";
+  let PrintMethod = "printUImm<8>";
 }
 
 def vsplat_uimm5 : Operand<vAny> {
-  let PrintMethod = "printUnsignedImm8";
+  let PrintMethod = "printUImm<8>";
 }
 
 def vsplat_uimm6 : Operand<vAny> {
-  let PrintMethod = "printUnsignedImm8";
+  let PrintMethod = "printUImm<8>";
 }
 
 def vsplat_uimm8 : Operand<vAny> {
-  let PrintMethod = "printUnsignedImm8";
+  let PrintMethod = "printUImm<8>";
 }
 
 def vsplat_simm5 : Operand<vAny>;

diff --git a/llvm/test/MC/Disassembler/Mips/micromips64r6/valid.txt b/llvm/test/MC/Disassembler/Mips/micromips64r6/valid.txt
@@ -25,8 +25,8 @@
 0x42 0x23 0x00 0x04 # CHECK: dahi $3, 4
 0x42 0x03 0x00 0x04 # CHECK: dati $3, 4
 0x59 0x26 0x30 0xec # CHECK: dext $9, $6, 3, 7
-0x59 0x26 0x30 0xe4 # CHECK: dextm $9, $6, 3, 7
-0x59 0x26 0x30 0xd4 # CHECK: dextu $9, $6, 3, 7
+0x59 0x26 0x30 0xe4 # CHECK: dextm $9, $6, 3, 39
+0x59 0x26 0x30 0xd4 # CHECK: dextu $9, $6, 35, 7
 0x58 0x43 0x25 0x1c # CHECK: dalign $4, $2, $3, 5
 0x58 0x64 0x29 0x18 # CHECK: ddiv $3, $4, $5
 0x58 0x64 0x29 0x58 # CHECK: dmod $3, $4, $5
@@ -171,6 +171,6 @@
 0x00 0x0f 0x47 0x7c # CHECK: di $15
 0x00 0x00 0x43 0x7c # CHECK: tlbinv
 0x00 0x00 0x53 0x7c # CHECK: tlbinvf
-0x58 0x82 0x20 0x34 # CHECK: dinsu $4, $2, 0, 5
+0x58 0x82 0x20 0x34 # CHECK: dinsu $4, $2, 32, 5
 0x58 0x82 0x38 0xc4 # CHECK: dinsm $4, $2, 3, 5
 0x58 0x82 0x38 0xcc # CHECK: dins $4, $2, 3, 5
diff --git a/llvm/test/MC/Mips/micromips/invalid-wrong-error.s b/llvm/test/MC/Mips/micromips/invalid-wrong-error.s
@@ -0,0 +1,12 @@
+# Instructions that are correctly rejected but emit a wrong or misleading error.
+# RUN: not llvm-mc %s -triple=mips -show-encoding -mattr=micromips 2>%t1
+# RUN: FileCheck %s < %t1
+
+  # The 20-bit immediate supported by the standard encodings cause us to emit
+  # the diagnostic for the 20-bit form. This isn't exactly wrong but it is
+  # misleading. Ideally, we'd emit every way to achieve a valid match instead
+  # of picking only one.
+  sdbbp -1            # CHECK: :[[@LINE]]:9: error: expected 20-bit unsigned immediate
+  sdbbp 1024          # CHECK: :[[@LINE]]:3: error: instruction requires a CPU feature not currently enabled
+  syscall -1          # CHECK: :[[@LINE]]:11: error: expected 20-bit unsigned immediate
+  syscall 1024        # CHECK: :[[@LINE]]:3: error: instruction requires a CPU feature not currently enabled