[llvm] 087b5f3 - [RISCV] Improve validation of opcode for .insn.

[Craig Topper via llvm-commits]

Author: Craig Topper
Date: 2023-03-21T16:20:30-07:00
New Revision: 087b5f3277f36225ad26cd265aace89150fe1f06

URL: https://github.com/llvm/llvm-project/commit/087b5f3277f36225ad26cd265aace89150fe1f06
DIFF: https://github.com/llvm/llvm-project/commit/087b5f3277f36225ad26cd265aace89150fe1f06.diff

LOG: [RISCV] Improve validation of opcode for .insn.

The lower 2 bits of the opcode must be 0x3. If the lower 2 bits are
0-2, it's a compressed instruction.

Merge 3 slightly different error messages into 1 to reduce code. The
messages differed slightly depending on whether we parsed a string
or an expression. The message gets a little more generic, but is no
more generic than what binutils prints.

Added: 
    

Modified: 
    llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
    llvm/test/MC/RISCV/insn-invalid.s

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index e8edc96951229..9c6d54e62b16c 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -1459,15 +1459,13 @@ RISCVAsmParser::parseInsnDirectiveOpcode(OperandVector &Operands) {
     auto *CE = dyn_cast<MCConstantExpr>(Res);
     if (CE) {
       int64_t Imm = CE->getValue();
-      if (isUInt<7>(Imm)) {
+      if (isUInt<7>(Imm) && (Imm & 3) == 3) {
         Operands.push_back(RISCVOperand::createImm(Res, S, E, isRV64()));
         return MatchOperand_Success;
       }
     }
 
-    Twine Msg = "immediate must be an integer in the range";
-    Error(S, Msg + " [" + Twine(0) + ", " + Twine((1 << 7) - 1) + "]");
-    return MatchOperand_ParseFail;
+    break;
   }
   case AsmToken::Identifier: {
     StringRef Identifier;
@@ -1476,26 +1474,23 @@ RISCVAsmParser::parseInsnDirectiveOpcode(OperandVector &Operands) {
 
     auto Opcode = RISCVInsnOpcode::lookupRISCVOpcodeByName(Identifier);
     if (Opcode) {
+      assert(isUInt<7>(Opcode->Value) && (Opcode->Value & 0x3) == 3 &&
+             "Unexpected opcode");
       Res = MCConstantExpr::create(Opcode->Value, getContext());
       E = SMLoc::getFromPointer(S.getPointer() + Identifier.size());
       Operands.push_back(RISCVOperand::createImm(Res, S, E, isRV64()));
       return MatchOperand_Success;
     }
 
-    Twine Msg = "operand must be a valid opcode name or an "
-                "integer in the range";
-    Error(S, Msg + " [" + Twine(0) + ", " + Twine((1 << 7) - 1) + "]");
-    return MatchOperand_ParseFail;
-  }
-  case AsmToken::Percent: {
-    // Discard operand with modifier.
-    Twine Msg = "immediate must be an integer in the range";
-    Error(S, Msg + " [" + Twine(0) + ", " + Twine((1 << 7) - 1) + "]");
-    return MatchOperand_ParseFail;
+    break;
   }
+  case AsmToken::Percent:
+    break;
   }
 
-  return MatchOperand_NoMatch;
+  Error(S, "opcode must be in the range [0, 127] and the lower 2 bits must be "
+           "0x3");
+  return MatchOperand_ParseFail;
 }
 
 OperandMatchResultTy

diff  --git a/llvm/test/MC/RISCV/insn-invalid.s b/llvm/test/MC/RISCV/insn-invalid.s
index c9142747f07db..ab41f07ef567f 100644
--- a/llvm/test/MC/RISCV/insn-invalid.s
+++ b/llvm/test/MC/RISCV/insn-invalid.s
@@ -14,12 +14,13 @@
 .insn q  0x13,  0,  a0, a1, 13, 14 # CHECK: :[[@LINE]]:7: error: invalid instruction format
 
 # Invalid immediate
-.insn i  0x99,  0, a0, 4(a1) # CHECK: :[[@LINE]]:10: error: immediate must be an integer in the range [0, 127]
+.insn i  0x99,  0, a0, 4(a1) # CHECK: :[[@LINE]]:10: error: opcode must be in the range [0, 127] and the lower 2 bits must be 0x3
+.insn i  0,  0, a0, 4(a1) # CHECK: :[[@LINE]]:10: error: opcode must be in the range [0, 127] and the lower 2 bits must be 0x3
 .insn r  0x33,  8,  0, a0, a1, a2 # CHECK: :[[@LINE]]:17: error: immediate must be an integer in the range [0, 7]
 .insn r4 0x43,  0,  4, fa0, fa1, fa2, fa3 # CHECK: :[[@LINE]]:21: error: immediate must be an integer in the range [0, 3]
 
 # Unrecognized opcode name
-.insn r UNKNOWN, 0, a1, a2, a3 #CHECK: :[[@LINE]]:9: error: operand must be a valid opcode name or an integer in the range [0, 127]
+.insn r UNKNOWN, 0, a1, a2, a3 #CHECK: :[[@LINE]]:9: error: opcode must be in the range [0, 127] and the lower 2 bits must be 0x3
 
 # Make fake mnemonics we use to match these in the tablegened asm match table isn't exposed.
 .insn_i  0x13,  0,  a0, a1, 13, 14 # CHECK: :[[@LINE]]:1: error: unknown directive