[RISCV] Correctly Decode Unsigned Immediates with Ranges #128584
Conversation
We currently have two operands upstream that are an unsigned immediate with a range constraint - `uimm8ge32` (for `cm.jalt`) and `uimm5gt3` (for `qc.shladd`). Both of these were using `decodeUImmOperand<N>` for decoding. For Zcmt this worked, because the generated decoder automatically checked for `cm.jt` first because the 8 undefined bits in `cm.jalt` are `000?????` in `cm.jt` (this is to do with the range lower-bound being a power-of-two). For Zcmt, this patch is NFC. We have less luck with `Xqciac` - `qc.shladd` is being decoded where the `uimm5` field is 3 or lower. This patch fixes this by introducing a `decodeUImmOperandGE<Width, LowerBound>` helper, which will corretly return `MCDisassembler::Fail` when the immediate is below the lower bound. I have added a test to show the encoding where `uimm5` is equal to 3 is no longer disassembled as `qc.shladd`.
|
@llvm/pr-subscribers-backend-risc-v Author: Sam Elliott (lenary) ChangesWe currently have two operands upstream that are an unsigned immediate with a range constraint - Both of these were using We have less luck with I have added a test to show the encoding where Full diff: https://github.com/llvm/llvm-project/pull/128584.diff 4 Files Affected:
diff --git a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
index 8c07d87680d65..830dc28a22175 100644
--- a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
+++ b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
@@ -328,6 +328,19 @@ static DecodeStatus decodeUImmOperand(MCInst &Inst, uint32_t Imm,
return MCDisassembler::Success;
}
+template <unsigned Width, unsigned LowerBound>
+static DecodeStatus decodeUImmOperandGE(MCInst &Inst, uint32_t Imm,
+ int64_t Address,
+ const MCDisassembler *Decoder) {
+ assert(isUInt<Width>(Imm) && "Invalid immediate");
+
+ if (Imm < LowerBound)
+ return MCDisassembler::Fail;
+
+ Inst.addOperand(MCOperand::createImm(Imm));
+ return MCDisassembler::Success;
+}
+
static DecodeStatus decodeUImmLog2XLenOperand(MCInst &Inst, uint32_t Imm,
int64_t Address,
const MCDisassembler *Decoder) {
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
index 3a8039fce1f49..d5bc1b20b510d 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
@@ -24,7 +24,7 @@ def uimm5nonzero : RISCVOp<XLenVT>,
def uimm5gt3 : RISCVOp<XLenVT>, ImmLeaf<XLenVT,
[{return (Imm > 3) && isUInt<5>(Imm);}]> {
let ParserMatchClass = UImmAsmOperand<5, "GT3">;
- let DecoderMethod = "decodeUImmOperand<5>";
+ let DecoderMethod = "decodeUImmOperandGE<5, 4>";
let OperandType = "OPERAND_UIMM5_GT3";
}
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td b/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
index 9dfbcf678d6eb..1740ebb239217 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
@@ -31,7 +31,7 @@ def uimm2_lsb0 : RISCVOp,
def uimm8ge32 : RISCVOp {
let ParserMatchClass = UImmAsmOperand<8, "GE32">;
- let DecoderMethod = "decodeUImmOperand<8>";
+ let DecoderMethod = "decodeUImmOperandGE<8, 32>";
let OperandType = "OPERAND_UIMM8_GE32";
}
diff --git a/llvm/test/MC/Disassembler/RISCV/xqci-invalid.txt b/llvm/test/MC/Disassembler/RISCV/xqci-invalid.txt
new file mode 100644
index 0000000000000..0acf42aa8717c
--- /dev/null
+++ b/llvm/test/MC/Disassembler/RISCV/xqci-invalid.txt
@@ -0,0 +1,10 @@
+# RUN: not llvm-mc -disassemble -triple=riscv32 -mattr=+experimental-xqciac %s | FileCheck %s
+
+[0x00,0x00]
+# CHECK: unimp
+
+[0x8b,0x30,0x31,0x46]
+# CHECK: qc.shladd x1, x2, x3, {{\d+}}
+
+[0x00,0x00]
+# CHECK: unimp
|
|
@llvm/pr-subscribers-mc Author: Sam Elliott (lenary) ChangesWe currently have two operands upstream that are an unsigned immediate with a range constraint - Both of these were using We have less luck with I have added a test to show the encoding where Full diff: https://github.com/llvm/llvm-project/pull/128584.diff 4 Files Affected:
diff --git a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
index 8c07d87680d65..830dc28a22175 100644
--- a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
+++ b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
@@ -328,6 +328,19 @@ static DecodeStatus decodeUImmOperand(MCInst &Inst, uint32_t Imm,
return MCDisassembler::Success;
}
+template <unsigned Width, unsigned LowerBound>
+static DecodeStatus decodeUImmOperandGE(MCInst &Inst, uint32_t Imm,
+ int64_t Address,
+ const MCDisassembler *Decoder) {
+ assert(isUInt<Width>(Imm) && "Invalid immediate");
+
+ if (Imm < LowerBound)
+ return MCDisassembler::Fail;
+
+ Inst.addOperand(MCOperand::createImm(Imm));
+ return MCDisassembler::Success;
+}
+
static DecodeStatus decodeUImmLog2XLenOperand(MCInst &Inst, uint32_t Imm,
int64_t Address,
const MCDisassembler *Decoder) {
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
index 3a8039fce1f49..d5bc1b20b510d 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
@@ -24,7 +24,7 @@ def uimm5nonzero : RISCVOp<XLenVT>,
def uimm5gt3 : RISCVOp<XLenVT>, ImmLeaf<XLenVT,
[{return (Imm > 3) && isUInt<5>(Imm);}]> {
let ParserMatchClass = UImmAsmOperand<5, "GT3">;
- let DecoderMethod = "decodeUImmOperand<5>";
+ let DecoderMethod = "decodeUImmOperandGE<5, 4>";
let OperandType = "OPERAND_UIMM5_GT3";
}
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td b/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
index 9dfbcf678d6eb..1740ebb239217 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
@@ -31,7 +31,7 @@ def uimm2_lsb0 : RISCVOp,
def uimm8ge32 : RISCVOp {
let ParserMatchClass = UImmAsmOperand<8, "GE32">;
- let DecoderMethod = "decodeUImmOperand<8>";
+ let DecoderMethod = "decodeUImmOperandGE<8, 32>";
let OperandType = "OPERAND_UIMM8_GE32";
}
diff --git a/llvm/test/MC/Disassembler/RISCV/xqci-invalid.txt b/llvm/test/MC/Disassembler/RISCV/xqci-invalid.txt
new file mode 100644
index 0000000000000..0acf42aa8717c
--- /dev/null
+++ b/llvm/test/MC/Disassembler/RISCV/xqci-invalid.txt
@@ -0,0 +1,10 @@
+# RUN: not llvm-mc -disassemble -triple=riscv32 -mattr=+experimental-xqciac %s | FileCheck %s
+
+[0x00,0x00]
+# CHECK: unimp
+
+[0x8b,0x30,0x31,0x46]
+# CHECK: qc.shladd x1, x2, x3, {{\d+}}
+
+[0x00,0x00]
+# CHECK: unimp
|
|
it really helps if i write the test so it checks what i intended |
I guess I only glanced at it and didn't think too hard about it. |
4 participants
We currently have two operands upstream that are an unsigned immediate with a range constraint -
uimm8ge32(forcm.jalt) anduimm5gt3(forqc.shladd).Both of these were using
decodeUImmOperand<N>for decoding. ForZcmtthis worked, because the generated decoder automatically checked forcm.jtfirst because the 8 undefined bits incm.jaltare000?????incm.jt(this is to do with the range lower-bound being a power-of-two). For Zcmt, this patch is NFC.We have less luck with
Xqciac-qc.shladdis being decoded where theuimm5field is 3 or lower. This patch fixes this by introducing adecodeUImmOperandGE<Width, LowerBound>helper, which will corretly returnMCDisassembler::Failwhen the immediate is below the lower bound.I have added a test to show the encoding where
uimm5is equal to 3 is no longer disassembled asqc.shladd.