Skip to content

[RISCV] Only allow 5 bit shift amounts in disassembler for RV32. #115432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 8, 2024

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Nov 8, 2024

Fixes 2 old TODOs

@llvmbot llvmbot added backend:RISC-V mc Machine (object) code labels Nov 8, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 8, 2024

@llvm/pr-subscribers-backend-risc-v

Author: Craig Topper (topperc)

Changes

Fixes 2 old TODOs


Full diff: https://github.com/llvm/llvm-project/pull/115432.diff

4 Files Affected:

  • (modified) llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp (+22)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.td (+1-2)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoC.td (+1-2)
  • (added) llvm/test/MC/Disassembler/RISCV/rv32-invalid-shift.txt (+10)
diff --git a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
index be0a61263297e4..d985b45495864a 100644
--- a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
+++ b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
@@ -314,6 +314,20 @@ static DecodeStatus decodeUImmOperand(MCInst &Inst, uint32_t Imm,
   return MCDisassembler::Success;
 }
 
+static DecodeStatus decodeUImmLog2XLenOperand(MCInst &Inst, uint32_t Imm,
+                                              int64_t Address,
+                                              const MCDisassembler *Decoder) {
+  assert(isUInt<6>(Imm) && "Invalid immediate");
+
+  if (!Decoder->getSubtargetInfo().hasFeature(RISCV::Feature64Bit) &&
+      !isUInt<5>(Imm))
+    return MCDisassembler::Fail;
+  ;
+
+  Inst.addOperand(MCOperand::createImm(Imm));
+  return MCDisassembler::Success;
+}
+
 template <unsigned N>
 static DecodeStatus decodeUImmNonZeroOperand(MCInst &Inst, uint32_t Imm,
                                              int64_t Address,
@@ -323,6 +337,14 @@ static DecodeStatus decodeUImmNonZeroOperand(MCInst &Inst, uint32_t Imm,
   return decodeUImmOperand<N>(Inst, Imm, Address, Decoder);
 }
 
+static DecodeStatus
+decodeUImmLog2XLenNonZeroOperand(MCInst &Inst, uint32_t Imm, int64_t Address,
+                                 const MCDisassembler *Decoder) {
+  if (Imm == 0)
+    return MCDisassembler::Fail;
+  return decodeUImmLog2XLenOperand(Inst, Imm, Address, Decoder);
+}
+
 template <unsigned N>
 static DecodeStatus decodeSImmOperand(MCInst &Inst, uint32_t Imm,
                                       int64_t Address,
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.td b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
index a867368235584c..1908f5e5dede85 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -201,8 +201,7 @@ def uimmlog2xlen : RISCVOp, ImmLeaf<XLenVT, [{
   return isUInt<5>(Imm);
 }]> {
   let ParserMatchClass = UImmLog2XLenAsmOperand;
-  // TODO: should ensure invalid shamt is rejected when decoding.
-  let DecoderMethod = "decodeUImmOperand<6>";
+  let DecoderMethod = "decodeUImmLog2XLenOperand";
   let MCOperandPredicate = [{
     int64_t Imm;
     if (!MCOp.evaluateAsConstantImm(Imm))
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
index 8a76dba23d42a7..e5a5f60f9fec10 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
@@ -24,8 +24,7 @@ def uimmlog2xlennonzero : RISCVOp, ImmLeaf<XLenVT, [{
   return isUInt<5>(Imm) && (Imm != 0);
 }]> {
   let ParserMatchClass = UImmLog2XLenNonZeroAsmOperand;
-  // TODO: should ensure invalid shamt is rejected when decoding.
-  let DecoderMethod = "decodeUImmNonZeroOperand<6>";
+  let DecoderMethod = "decodeUImmLog2XLenNonZeroOperand";
   let OperandType = "OPERAND_UIMMLOG2XLEN_NONZERO";
   let MCOperandPredicate = [{
     int64_t Imm;
diff --git a/llvm/test/MC/Disassembler/RISCV/rv32-invalid-shift.txt b/llvm/test/MC/Disassembler/RISCV/rv32-invalid-shift.txt
new file mode 100644
index 00000000000000..f56530190adc81
--- /dev/null
+++ b/llvm/test/MC/Disassembler/RISCV/rv32-invalid-shift.txt
@@ -0,0 +1,10 @@
+# RUN: not llvm-mc -disassemble -triple=riscv32 -mattr=+c < %s 2>&1 | FileCheck %s --check-prefix=RV32
+# RUN: llvm-mc -disassemble -triple=riscv64 -mattr=+c < %s 2>&1 | FileCheck %s --check-prefix=RV64
+
+[0x13,0x9b,0xdb,0x02]
+# RV32: warning: invalid instruction encoding
+# RV64: slli    s6, s7, 45
+
+[0xfd,0x92]
+# RV32: warning: invalid instruction encoding
+# RV64: srli    a3, a3, 63

@llvmbot
Copy link
Member

llvmbot commented Nov 8, 2024

@llvm/pr-subscribers-mc

Author: Craig Topper (topperc)

Changes

Fixes 2 old TODOs


Full diff: https://github.com/llvm/llvm-project/pull/115432.diff

4 Files Affected:

  • (modified) llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp (+22)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.td (+1-2)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoC.td (+1-2)
  • (added) llvm/test/MC/Disassembler/RISCV/rv32-invalid-shift.txt (+10)
diff --git a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
index be0a61263297e4..d985b45495864a 100644
--- a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
+++ b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
@@ -314,6 +314,20 @@ static DecodeStatus decodeUImmOperand(MCInst &Inst, uint32_t Imm,
   return MCDisassembler::Success;
 }
 
+static DecodeStatus decodeUImmLog2XLenOperand(MCInst &Inst, uint32_t Imm,
+                                              int64_t Address,
+                                              const MCDisassembler *Decoder) {
+  assert(isUInt<6>(Imm) && "Invalid immediate");
+
+  if (!Decoder->getSubtargetInfo().hasFeature(RISCV::Feature64Bit) &&
+      !isUInt<5>(Imm))
+    return MCDisassembler::Fail;
+  ;
+
+  Inst.addOperand(MCOperand::createImm(Imm));
+  return MCDisassembler::Success;
+}
+
 template <unsigned N>
 static DecodeStatus decodeUImmNonZeroOperand(MCInst &Inst, uint32_t Imm,
                                              int64_t Address,
@@ -323,6 +337,14 @@ static DecodeStatus decodeUImmNonZeroOperand(MCInst &Inst, uint32_t Imm,
   return decodeUImmOperand<N>(Inst, Imm, Address, Decoder);
 }
 
+static DecodeStatus
+decodeUImmLog2XLenNonZeroOperand(MCInst &Inst, uint32_t Imm, int64_t Address,
+                                 const MCDisassembler *Decoder) {
+  if (Imm == 0)
+    return MCDisassembler::Fail;
+  return decodeUImmLog2XLenOperand(Inst, Imm, Address, Decoder);
+}
+
 template <unsigned N>
 static DecodeStatus decodeSImmOperand(MCInst &Inst, uint32_t Imm,
                                       int64_t Address,
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.td b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
index a867368235584c..1908f5e5dede85 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -201,8 +201,7 @@ def uimmlog2xlen : RISCVOp, ImmLeaf<XLenVT, [{
   return isUInt<5>(Imm);
 }]> {
   let ParserMatchClass = UImmLog2XLenAsmOperand;
-  // TODO: should ensure invalid shamt is rejected when decoding.
-  let DecoderMethod = "decodeUImmOperand<6>";
+  let DecoderMethod = "decodeUImmLog2XLenOperand";
   let MCOperandPredicate = [{
     int64_t Imm;
     if (!MCOp.evaluateAsConstantImm(Imm))
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
index 8a76dba23d42a7..e5a5f60f9fec10 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
@@ -24,8 +24,7 @@ def uimmlog2xlennonzero : RISCVOp, ImmLeaf<XLenVT, [{
   return isUInt<5>(Imm) && (Imm != 0);
 }]> {
   let ParserMatchClass = UImmLog2XLenNonZeroAsmOperand;
-  // TODO: should ensure invalid shamt is rejected when decoding.
-  let DecoderMethod = "decodeUImmNonZeroOperand<6>";
+  let DecoderMethod = "decodeUImmLog2XLenNonZeroOperand";
   let OperandType = "OPERAND_UIMMLOG2XLEN_NONZERO";
   let MCOperandPredicate = [{
     int64_t Imm;
diff --git a/llvm/test/MC/Disassembler/RISCV/rv32-invalid-shift.txt b/llvm/test/MC/Disassembler/RISCV/rv32-invalid-shift.txt
new file mode 100644
index 00000000000000..f56530190adc81
--- /dev/null
+++ b/llvm/test/MC/Disassembler/RISCV/rv32-invalid-shift.txt
@@ -0,0 +1,10 @@
+# RUN: not llvm-mc -disassemble -triple=riscv32 -mattr=+c < %s 2>&1 | FileCheck %s --check-prefix=RV32
+# RUN: llvm-mc -disassemble -triple=riscv64 -mattr=+c < %s 2>&1 | FileCheck %s --check-prefix=RV64
+
+[0x13,0x9b,0xdb,0x02]
+# RV32: warning: invalid instruction encoding
+# RV64: slli    s6, s7, 45
+
+[0xfd,0x92]
+# RV32: warning: invalid instruction encoding
+# RV64: srli    a3, a3, 63

Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Co-authored-by: Yingwei Zheng <[email protected]>
@topperc topperc merged commit bde3d4a into llvm:main Nov 8, 2024
8 checks passed
@topperc topperc deleted the pr/shift-amount branch November 8, 2024 17:12
Groverkss pushed a commit to iree-org/llvm-project that referenced this pull request Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants