[RISCV] Fix c.addi and c.addi16sp immediate constraints which should be non-zero

[shiva0217]

Summary

Fix c.addi and c.addi16sp immediate constraints which should be non-zero in the range.

[chenwj]

Comment inline.

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

358

Nitpick. Wouldn't be better testing Imm is zero once we know Imm is a constant?

return IsConstantImm && (Imm != 0) && isShiftedInt<6, 4>(Imm) &&
            VK == RISCVMCExpr::VK_RISCV_None;

lib/Target/RISCV/RISCVInstrInfoC.td

103

Same here.

[asb]

Looks good to me - thanks!

[apazos]

Thanks Shiva, this solves the QEMU crashes.

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

bool isSImm6() const {
     if (!IsConstantImm)
       IsValid = RISCVAsmParser::classifySymbolRef(getImm(), VK, Imm);
     else
       IsValid = isInt<6>(Imm);
     return IsValid &&
            (VK == RISCVMCExpr::VK_RISCV_None || VK == RISCVMCExpr::VK_RISCV_LO);
   }
 
+  bool isSImm6NonZero() const {
+    RISCVMCExpr::VariantKind VK;
+    int64_t Imm;
+    bool IsValid;
+    bool IsConstantImm = evaluateConstantImm(Imm, VK);
+    if (!IsConstantImm)
+      IsValid = RISCVAsmParser::classifySymbolRef(getImm(), VK, Imm);
+    else
+      IsValid = (isInt<6>(Imm) && (Imm != 0));
+    return IsValid &&
+           (VK == RISCVMCExpr::VK_RISCV_None || VK == RISCVMCExpr::VK_RISCV_LO);
+  }
+
   bool isUImm6NonZero() const {
     int64_t Imm;
     RISCVMCExpr::VariantKind VK;
     bool IsConstantImm = evaluateConstantImm(Imm, VK);
     return IsConstantImm && isUInt<6>(Imm) && (Imm != 0) &&
            VK == RISCVMCExpr::VK_RISCV_None;
   }
 
bool isUImm12() const {
     if (!isImm())
       return false;
     bool IsConstantImm = evaluateConstantImm(Imm, VK);
     return IsConstantImm && isUInt<12>(Imm) && VK == RISCVMCExpr::VK_RISCV_None;
   }
 
   bool isSImm13Lsb0() const { return isBareSimmNLsb0<13>(); }
 
-  bool isSImm10Lsb0000() const {
+  bool isSImm10Lsb0000NonZero() const {
     int64_t Imm;
     RISCVMCExpr::VariantKind VK;
     bool IsConstantImm = evaluateConstantImm(Imm, VK);
-    return IsConstantImm && isShiftedInt<6, 4>(Imm) &&
+    return IsConstantImm && isShiftedInt<6, 4>(Imm) && (Imm != 0) &&
            VK == RISCVMCExpr::VK_RISCV_None;
   }
 
   bool isUImm20() const {
     RISCVMCExpr::VariantKind VK;
     int64_t Imm;
     bool IsValid;
     if (!isImm())
case Match_InvalidUImmLog2XLenNonZero:
     if (isRV64())
       return generateImmOutOfRangeError(Operands, ErrorInfo, 1, (1 << 6) - 1);
     return generateImmOutOfRangeError(Operands, ErrorInfo, 1, (1 << 5) - 1);
   case Match_InvalidUImm5:
     return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 5) - 1);
   case Match_InvalidSImm6:
     return generateImmOutOfRangeError(Operands, ErrorInfo, -(1 << 5),
                                       (1 << 5) - 1);
+  case Match_InvalidSImm6NonZero:
+    return generateImmOutOfRangeError(Operands, ErrorInfo, -(1 << 5),
+                                      (1 << 5) - 1,
+        "immediate must be non-zero in the range");
   case Match_InvalidUImm6NonZero:
     return generateImmOutOfRangeError(Operands, ErrorInfo, 1, (1 << 6) - 1);
   case Match_InvalidUImm7Lsb00:
     return generateImmOutOfRangeError(
         Operands, ErrorInfo, 0, (1 << 7) - 4,
         "immediate must be a multiple of 4 bytes in the range");
   case Match_InvalidUImm8Lsb00:
     return generateImmOutOfRangeError(
bool RISCVAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
   case Match_InvalidUImm9Lsb000:
     return generateImmOutOfRangeError(
         Operands, ErrorInfo, 0, (1 << 9) - 8,
         "immediate must be a multiple of 8 bytes in the range");
   case Match_InvalidUImm10Lsb00NonZero:
     return generateImmOutOfRangeError(
         Operands, ErrorInfo, 4, (1 << 10) - 4,
         "immediate must be a multiple of 4 bytes in the range");
-  case Match_InvalidSImm10Lsb0000:
+  case Match_InvalidSImm10Lsb0000NonZero:
     return generateImmOutOfRangeError(
         Operands, ErrorInfo, -(1 << 9), (1 << 9) - 16,
-        "immediate must be a multiple of 16 bytes in the range");
+        "immediate must be a multiple of 16 bytes and non-zero in the range");
   case Match_InvalidSImm12:
     return generateImmOutOfRangeError(Operands, ErrorInfo, -(1 << 11),
                                       (1 << 11) - 1);
   case Match_InvalidSImm12Lsb0:
     return generateImmOutOfRangeError(
         Operands, ErrorInfo, -(1 << 11), (1 << 11) - 2,
         "immediate must be a multiple of 2 bytes in the range");
   case Match_InvalidUImm12:

lib/Target/RISCV/RISCVInstrInfoC.td

 }
 
 def simm6 : Operand<XLenVT>, ImmLeaf<XLenVT, [{return isInt<6>(Imm);}]> {
   let ParserMatchClass = SImmAsmOperand<6>;
   let EncoderMethod = "getImmOpValue";
   let DecoderMethod = "decodeSImmOperand<6>";
 }
 
+def simm6nonzero : Operand<XLenVT>,
+                   ImmLeaf<XLenVT, [{return isInt<6>(Imm) && (Imm != 0);}]> {
+  let ParserMatchClass = SImmAsmOperand<6, "NonZero">;
+  let EncoderMethod = "getImmOpValue";
+  let DecoderMethod = "decodeSImmOperand<6>";
+}
+
 def uimm6nonzero : Operand<XLenVT>,
                    ImmLeaf<XLenVT, [{return isUInt<6>(Imm) && (Imm != 0);}]> {
   let ParserMatchClass = UImmAsmOperand<6, "NonZero">;
   let DecoderMethod = "decodeUImmOperand<6>";
 }
 
 // A 7-bit unsigned immediate where the least significant two bits are zero.
 def uimm7_lsb00 : Operand<XLenVT>,
def uimm10_lsb00nonzero : Operand<XLenVT>,
                           ImmLeaf<XLenVT,
                           [{return isShiftedUInt<8, 2>(Imm) && (Imm != 0);}]> {
   let ParserMatchClass = UImmAsmOperand<10, "Lsb00NonZero">;
   let EncoderMethod = "getImmOpValue";
   let DecoderMethod = "decodeUImmOperand<10>";
 }
 
 // A 10-bit signed immediate where the least significant four bits are zero.
-def simm10_lsb0000 : Operand<XLenVT>,
-                     ImmLeaf<XLenVT, [{return isShiftedInt<6, 4>(Imm);}]> {
-  let ParserMatchClass = SImmAsmOperand<10, "Lsb0000">;
+def simm10_lsb0000nonzero : Operand<XLenVT>,
+                            ImmLeaf<XLenVT,
+                            [{return isShiftedInt<6, 4>(Imm) && (Imm != 0);}]> {
+  let ParserMatchClass = SImmAsmOperand<10, "Lsb0000NonZero">;
   let EncoderMethod = "getImmOpValue";
   let DecoderMethod = "decodeSImmOperand<10>";
 }
 
 // A 12-bit signed immediate where the least significant bit is zero.
 def simm12_lsb0 : Operand<OtherVT> {
   let ParserMatchClass = SImmAsmOperand<12, "Lsb0">;
   let EncoderMethod = "getImmOpValueAsr1";
def C_SD : CStore_rri<0b111, "c.sd", GPRC, uimm8_lsb000> {
   let Inst{6-5} = imm{7-6};
 }
 
 let rd = 0, imm = 0, hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
 def C_NOP : RVInst16CI<0b000, 0b01, (outs), (ins), "c.nop", "">;
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
 def C_ADDI : RVInst16CI<0b000, 0b01, (outs GPRNoX0:$rd_wb),
-                        (ins GPRNoX0:$rd, simm6:$imm),
+                        (ins GPRNoX0:$rd, simm6nonzero:$imm),
                         "c.addi", "$rd, $imm"> {
   let Constraints = "$rd = $rd_wb";
   let Inst{6-2} = imm{4-0};
 }
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0, isCall = 1,
     DecoderNamespace = "RISCV32Only_", Defs = [X1],
     Predicates = [HasStdExtC, IsRV32]  in

 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
 def C_LI : RVInst16CI<0b010, 0b01, (outs GPRNoX0:$rd), (ins simm6:$imm),
                       "c.li", "$rd, $imm"> {
   let Inst{6-2} = imm{4-0};
 }
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
 def C_ADDI16SP : RVInst16CI<0b011, 0b01, (outs SP:$rd_wb),
-                            (ins SP:$rd, simm10_lsb0000:$imm),
+                            (ins SP:$rd, simm10_lsb0000nonzero:$imm),
                             "c.addi16sp", "$rd, $imm"> {
   let Constraints = "$rd = $rd_wb";
   let Inst{12} = imm{9};
   let Inst{11-7} = 2;
   let Inst{6} = imm{4};
   let Inst{5} = imm{6};
   let Inst{4-3} = imm{8-7};
   let Inst{2} = imm{5};

test/MC/RISCV/rv32c-invalid.s

 
 ## uimmlog2xlennonzero
 c.slli t0, 64 # CHECK: :[[@LINE]]:12: error: immediate must be an integer in the range [1, 31]
 c.srli a0, 32 # CHECK: :[[@LINE]]:12: error: immediate must be an integer in the range [1, 31]
 c.srai a0, 0  # CHECK: :[[@LINE]]:12: error: immediate must be an integer in the range [1, 31]
 
 ## simm6
 c.li t0, 128 # CHECK: :[[@LINE]]:10: error: immediate must be an integer in the range [-32, 31]
-c.addi t0, 32 # CHECK: :[[@LINE]]:12: error: immediate must be an integer in the range [-32, 31]
 c.andi a0, -33 # CHECK: :[[@LINE]]:12: error: immediate must be an integer in the range [-32, 31]
 
+## simm6nonzero
+c.addi t0, 0 # CHECK: :[[@LINE]]:12: error: immediate must be non-zero in the range [-32, 31]
+c.addi t0, -33 # CHECK: :[[@LINE]]:12: error: immediate must be non-zero in the range [-32, 31]
+c.addi t0, 32 # CHECK: :[[@LINE]]:12: error: immediate must be non-zero in the range [-32, 31]
+
 ## uimm6nonzero
 c.lui t0, 64 # CHECK: :[[@LINE]]:11: error: immediate must be an integer in the range [1, 63]
 c.lui t0, 0 # CHECK: :[[@LINE]]:11: error: immediate must be an integer in the range [1, 63]
 
 ## uimm8_lsb00
 c.lwsp  ra, 256(sp) # CHECK: :[[@LINE]]:13: error: immediate must be a multiple of 4 bytes in the range [0, 252]
 c.swsp  ra, -4(sp) # CHECK: :[[@LINE]]:13: error: immediate must be a multiple of 4 bytes in the range [0, 252]
 ## uimm7_lsb00
 c.lw  s0, -4(sp) # CHECK: :[[@LINE]]:11: error: immediate must be a multiple of 4 bytes in the range [0, 124]
 c.sw  s0, 128(sp) # CHECK: :[[@LINE]]:11: error: immediate must be a multiple of 4 bytes in the range [0, 124]
 
 ## simm9_lsb0
 c.bnez  s1, -258 # CHECK: :[[@LINE]]:13: error: immediate must be a multiple of 2 bytes in the range [-256, 254]
 c.beqz  a0, 256 # CHECK: :[[@LINE]]:13: error: immediate must be a multiple of 2 bytes in the range [-256, 254]
 
 ## simm12_lsb0
 c.j 2048 # CHECK: :[[@LINE]]:5: error: immediate must be a multiple of 2 bytes in the range [-2048, 2046]
 c.jal -2050 # CHECK: :[[@LINE]]:7: error: immediate must be a multiple of 2 bytes in the range [-2048, 2046]
 
 ## uimm10_lsb00nonzero
 c.addi4spn  a0, sp, 0 # CHECK: :[[@LINE]]:21: error: immediate must be a multiple of 4 bytes in the range [4, 1020]
 c.addi4spn  a0, sp, 1024 # CHECK: :[[@LINE]]:21: error: immediate must be a multiple of 4 bytes in the range [4, 1020]
 
-## simm10_lsb0000
-c.addi16sp  sp, -528 # CHECK: :[[@LINE]]:17: error: immediate must be a multiple of 16 bytes in the range [-512, 496]
-c.addi16sp  sp, 512 # CHECK: :[[@LINE]]:17: error: immediate must be a multiple of 16 bytes in the range [-512, 496]
+## simm10_lsb0000nonzero
+c.addi16sp  sp, -528 # CHECK: :[[@LINE]]:17: error: immediate must be a multiple of 16 bytes and non-zero in the range [-512, 496]
+c.addi16sp  sp, 512 # CHECK: :[[@LINE]]:17: error: immediate must be a multiple of 16 bytes and non-zero in the range [-512, 496]
+c.addi16sp  sp, 0 # CHECK: :[[@LINE]]:17: error: immediate must be a multiple of 16 bytes and non-zero in the range [-512, 496]