[llvm] ed3597e - [TableGen][Decoder] Decode operands with zero width or all bits known (#156358)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 4 07:48:40 PDT 2025
Author: Sergei Barannikov
Date: 2025-09-04T14:48:36Z
New Revision: ed3597e2f7c4a5ae1ad70de81032f2addf78e267
URL: https://github.com/llvm/llvm-project/commit/ed3597e2f7c4a5ae1ad70de81032f2addf78e267
DIFF: https://github.com/llvm/llvm-project/commit/ed3597e2f7c4a5ae1ad70de81032f2addf78e267.diff
LOG: [TableGen][Decoder] Decode operands with zero width or all bits known (#156358)
There are two classes of operands that DecoderEmitter cannot currently
handle:
1. Operands that do not participate in instruction encoding.
2. Operands whose encoding contains only 1s and 0s.
Because of this, targets developed various workarounds. Some targets
insert missing operands after an instruction has been (incompletely)
decoded, other take into account the missing operands when printing the
instruction. Some targets do neither of that and fail to correctly
disassemble some instructions.
This patch makes it possible to decode both classes of operands and
allows to remove existing workarounds.
For the case of operand with no contribution to instruction encoding,
one should now add `bits<0> OpName` field to instruction encoding
record. This will make DecoderEmitter generate a call to the decoder
function specified by the operand's DecoderMethod. The function has a
signature different from the usual one and looks like this:
```
static DecodeStatus DecodeImm42Operand(MCInst &Inst, const MCDisassembler *Decoder) {
Inst.addOperand(MCOperand::createImm(42));
return DecodeStatus::Success;
}
```
Notably, encoding bits are not passed to it (since there are none).
There is nothing special about the second case, the operand bits are
passed as usual. The difference is that before this change, the function
was not called if all the bits of the operand were known (no '?' in the
operand encoding).
There are two options controlling the behavior. Passing an option
enables the old behavior. They exist to allow smooth transition to the
new behavior. They are temporary (yeah, I know) and will be removed once
all targets migrate, possibly giving some more time to downstream
targets.
Subsequent patches in the stack enable the new behavior on some in-tree
targets.
Added:
Modified:
llvm/lib/Target/AArch64/CMakeLists.txt
llvm/lib/Target/AMDGPU/CMakeLists.txt
llvm/lib/Target/ARM/CMakeLists.txt
llvm/lib/Target/AVR/CMakeLists.txt
llvm/lib/Target/BPF/CMakeLists.txt
llvm/lib/Target/CSKY/CMakeLists.txt
llvm/lib/Target/Hexagon/CMakeLists.txt
llvm/lib/Target/Hexagon/HexagonDepInstrFormats.td
llvm/lib/Target/M68k/Disassembler/M68kDisassembler.cpp
llvm/lib/Target/Mips/CMakeLists.txt
llvm/lib/Target/RISCV/CMakeLists.txt
llvm/test/TableGen/AsmPredicateCombining.td
llvm/utils/TableGen/DecoderEmitter.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Target/AArch64/CMakeLists.txt b/llvm/lib/Target/AArch64/CMakeLists.txt
index 803943fd57c4d..833ce48ea1d7a 100644
--- a/llvm/lib/Target/AArch64/CMakeLists.txt
+++ b/llvm/lib/Target/AArch64/CMakeLists.txt
@@ -7,7 +7,9 @@ tablegen(LLVM AArch64GenAsmWriter.inc -gen-asm-writer)
tablegen(LLVM AArch64GenAsmWriter1.inc -gen-asm-writer -asmwriternum=1)
tablegen(LLVM AArch64GenCallingConv.inc -gen-callingconv)
tablegen(LLVM AArch64GenDAGISel.inc -gen-dag-isel)
-tablegen(LLVM AArch64GenDisassemblerTables.inc -gen-disassembler)
+tablegen(LLVM AArch64GenDisassemblerTables.inc -gen-disassembler
+ -ignore-non-decodable-operands
+ -ignore-fully-defined-operands)
tablegen(LLVM AArch64GenFastISel.inc -gen-fast-isel)
tablegen(LLVM AArch64GenGlobalISel.inc -gen-global-isel)
tablegen(LLVM AArch64GenO0PreLegalizeGICombiner.inc -gen-global-isel-combiner
diff --git a/llvm/lib/Target/AMDGPU/CMakeLists.txt b/llvm/lib/Target/AMDGPU/CMakeLists.txt
index 05295ae73be23..a915c4076ca2a 100644
--- a/llvm/lib/Target/AMDGPU/CMakeLists.txt
+++ b/llvm/lib/Target/AMDGPU/CMakeLists.txt
@@ -7,7 +7,9 @@ tablegen(LLVM AMDGPUGenAsmWriter.inc -gen-asm-writer)
tablegen(LLVM AMDGPUGenCallingConv.inc -gen-callingconv)
tablegen(LLVM AMDGPUGenDAGISel.inc -gen-dag-isel)
tablegen(LLVM AMDGPUGenDisassemblerTables.inc -gen-disassembler
- --specialize-decoders-per-bitwidth)
+ --specialize-decoders-per-bitwidth
+ -ignore-non-decodable-operands
+ -ignore-fully-defined-operands)
tablegen(LLVM AMDGPUGenInstrInfo.inc -gen-instr-info)
tablegen(LLVM AMDGPUGenMCCodeEmitter.inc -gen-emitter)
tablegen(LLVM AMDGPUGenMCPseudoLowering.inc -gen-pseudo-lowering)
diff --git a/llvm/lib/Target/ARM/CMakeLists.txt b/llvm/lib/Target/ARM/CMakeLists.txt
index a39629bd8aeb0..fa778cad4af8e 100644
--- a/llvm/lib/Target/ARM/CMakeLists.txt
+++ b/llvm/lib/Target/ARM/CMakeLists.txt
@@ -6,7 +6,8 @@ tablegen(LLVM ARMGenAsmMatcher.inc -gen-asm-matcher)
tablegen(LLVM ARMGenAsmWriter.inc -gen-asm-writer)
tablegen(LLVM ARMGenCallingConv.inc -gen-callingconv)
tablegen(LLVM ARMGenDAGISel.inc -gen-dag-isel)
-tablegen(LLVM ARMGenDisassemblerTables.inc -gen-disassembler)
+tablegen(LLVM ARMGenDisassemblerTables.inc -gen-disassembler
+ -ignore-non-decodable-operands)
tablegen(LLVM ARMGenFastISel.inc -gen-fast-isel)
tablegen(LLVM ARMGenGlobalISel.inc -gen-global-isel)
tablegen(LLVM ARMGenInstrInfo.inc -gen-instr-info)
diff --git a/llvm/lib/Target/AVR/CMakeLists.txt b/llvm/lib/Target/AVR/CMakeLists.txt
index a31c545f48ba3..2d5cb7e048778 100644
--- a/llvm/lib/Target/AVR/CMakeLists.txt
+++ b/llvm/lib/Target/AVR/CMakeLists.txt
@@ -6,7 +6,8 @@ tablegen(LLVM AVRGenAsmMatcher.inc -gen-asm-matcher)
tablegen(LLVM AVRGenAsmWriter.inc -gen-asm-writer)
tablegen(LLVM AVRGenCallingConv.inc -gen-callingconv)
tablegen(LLVM AVRGenDAGISel.inc -gen-dag-isel)
-tablegen(LLVM AVRGenDisassemblerTables.inc -gen-disassembler)
+tablegen(LLVM AVRGenDisassemblerTables.inc -gen-disassembler
+ -ignore-non-decodable-operands)
tablegen(LLVM AVRGenInstrInfo.inc -gen-instr-info)
tablegen(LLVM AVRGenMCCodeEmitter.inc -gen-emitter)
tablegen(LLVM AVRGenRegisterInfo.inc -gen-register-info)
diff --git a/llvm/lib/Target/BPF/CMakeLists.txt b/llvm/lib/Target/BPF/CMakeLists.txt
index eade4cacb7100..678cb42c35f13 100644
--- a/llvm/lib/Target/BPF/CMakeLists.txt
+++ b/llvm/lib/Target/BPF/CMakeLists.txt
@@ -6,7 +6,8 @@ tablegen(LLVM BPFGenAsmMatcher.inc -gen-asm-matcher)
tablegen(LLVM BPFGenAsmWriter.inc -gen-asm-writer)
tablegen(LLVM BPFGenCallingConv.inc -gen-callingconv)
tablegen(LLVM BPFGenDAGISel.inc -gen-dag-isel)
-tablegen(LLVM BPFGenDisassemblerTables.inc -gen-disassembler)
+tablegen(LLVM BPFGenDisassemblerTables.inc -gen-disassembler
+ -ignore-non-decodable-operands)
tablegen(LLVM BPFGenInstrInfo.inc -gen-instr-info)
tablegen(LLVM BPFGenMCCodeEmitter.inc -gen-emitter)
tablegen(LLVM BPFGenRegisterInfo.inc -gen-register-info)
diff --git a/llvm/lib/Target/CSKY/CMakeLists.txt b/llvm/lib/Target/CSKY/CMakeLists.txt
index 4b900bc99c271..101b34ea18442 100644
--- a/llvm/lib/Target/CSKY/CMakeLists.txt
+++ b/llvm/lib/Target/CSKY/CMakeLists.txt
@@ -7,7 +7,8 @@ tablegen(LLVM CSKYGenAsmWriter.inc -gen-asm-writer)
tablegen(LLVM CSKYGenCallingConv.inc -gen-callingconv)
tablegen(LLVM CSKYGenCompressInstEmitter.inc -gen-compress-inst-emitter)
tablegen(LLVM CSKYGenDAGISel.inc -gen-dag-isel)
-tablegen(LLVM CSKYGenDisassemblerTables.inc -gen-disassembler)
+tablegen(LLVM CSKYGenDisassemblerTables.inc -gen-disassembler
+ -ignore-non-decodable-operands)
tablegen(LLVM CSKYGenInstrInfo.inc -gen-instr-info)
tablegen(LLVM CSKYGenMCCodeEmitter.inc -gen-emitter)
tablegen(LLVM CSKYGenMCPseudoLowering.inc -gen-pseudo-lowering)
diff --git a/llvm/lib/Target/Hexagon/CMakeLists.txt b/llvm/lib/Target/Hexagon/CMakeLists.txt
index d758260a8ab5d..b615536af03be 100644
--- a/llvm/lib/Target/Hexagon/CMakeLists.txt
+++ b/llvm/lib/Target/Hexagon/CMakeLists.txt
@@ -7,7 +7,8 @@ tablegen(LLVM HexagonGenAsmWriter.inc -gen-asm-writer)
tablegen(LLVM HexagonGenCallingConv.inc -gen-callingconv)
tablegen(LLVM HexagonGenDAGISel.inc -gen-dag-isel)
tablegen(LLVM HexagonGenDFAPacketizer.inc -gen-dfa-packetizer)
-tablegen(LLVM HexagonGenDisassemblerTables.inc -gen-disassembler)
+tablegen(LLVM HexagonGenDisassemblerTables.inc -gen-disassembler
+ -ignore-non-decodable-operands)
tablegen(LLVM HexagonGenInstrInfo.inc -gen-instr-info)
tablegen(LLVM HexagonGenMCCodeEmitter.inc -gen-emitter)
tablegen(LLVM HexagonGenRegisterInfo.inc -gen-register-info)
diff --git a/llvm/lib/Target/Hexagon/HexagonDepInstrFormats.td b/llvm/lib/Target/Hexagon/HexagonDepInstrFormats.td
index 75e87c95f2c48..661b948346126 100644
--- a/llvm/lib/Target/Hexagon/HexagonDepInstrFormats.td
+++ b/llvm/lib/Target/Hexagon/HexagonDepInstrFormats.td
@@ -3049,7 +3049,6 @@ class Enc_cf1927 : OpcodeHexagon {
class Enc_d0fe02 : OpcodeHexagon {
bits <5> Rxx32;
let Inst{20-16} = Rxx32{4-0};
- bits <0> sgp10;
}
class Enc_d15d19 : OpcodeHexagon {
bits <1> Mu2;
diff --git a/llvm/lib/Target/M68k/Disassembler/M68kDisassembler.cpp b/llvm/lib/Target/M68k/Disassembler/M68kDisassembler.cpp
index d3ad65390143b..4992f1abe5a00 100644
--- a/llvm/lib/Target/M68k/Disassembler/M68kDisassembler.cpp
+++ b/llvm/lib/Target/M68k/Disassembler/M68kDisassembler.cpp
@@ -107,6 +107,18 @@ static DecodeStatus DecodeFPCSCRegisterClass(MCInst &Inst, uint64_t RegNo,
}
#define DecodeFPICRegisterClass DecodeFPCSCRegisterClass
+static DecodeStatus DecodeCCRCRegisterClass(MCInst &Inst,
+ const MCDisassembler *Decoder) {
+ Inst.addOperand(MCOperand::createReg(M68k::CCR));
+ return DecodeStatus::Success;
+}
+
+static DecodeStatus DecodeSRCRegisterClass(MCInst &Inst,
+ const MCDisassembler *Decoder) {
+ Inst.addOperand(MCOperand::createReg(M68k::SR));
+ return DecodeStatus::Success;
+}
+
static DecodeStatus DecodeImm32(MCInst &Inst, uint64_t Imm, uint64_t Address,
const void *Decoder) {
Inst.addOperand(MCOperand::createImm(M68k::swapWord<uint32_t>(Imm)));
diff --git a/llvm/lib/Target/Mips/CMakeLists.txt b/llvm/lib/Target/Mips/CMakeLists.txt
index 21d1765107ae6..4a2277e9a80dc 100644
--- a/llvm/lib/Target/Mips/CMakeLists.txt
+++ b/llvm/lib/Target/Mips/CMakeLists.txt
@@ -6,7 +6,8 @@ tablegen(LLVM MipsGenAsmMatcher.inc -gen-asm-matcher)
tablegen(LLVM MipsGenAsmWriter.inc -gen-asm-writer)
tablegen(LLVM MipsGenCallingConv.inc -gen-callingconv)
tablegen(LLVM MipsGenDAGISel.inc -gen-dag-isel)
-tablegen(LLVM MipsGenDisassemblerTables.inc -gen-disassembler)
+tablegen(LLVM MipsGenDisassemblerTables.inc -gen-disassembler
+ -ignore-non-decodable-operands)
tablegen(LLVM MipsGenFastISel.inc -gen-fast-isel)
tablegen(LLVM MipsGenGlobalISel.inc -gen-global-isel)
tablegen(LLVM MipsGenPostLegalizeGICombiner.inc -gen-global-isel-combiner
diff --git a/llvm/lib/Target/RISCV/CMakeLists.txt b/llvm/lib/Target/RISCV/CMakeLists.txt
index 531238ae85029..720361dc3da5b 100644
--- a/llvm/lib/Target/RISCV/CMakeLists.txt
+++ b/llvm/lib/Target/RISCV/CMakeLists.txt
@@ -8,7 +8,8 @@ tablegen(LLVM RISCVGenCompressInstEmitter.inc -gen-compress-inst-emitter)
tablegen(LLVM RISCVGenMacroFusion.inc -gen-macro-fusion-pred)
tablegen(LLVM RISCVGenDAGISel.inc -gen-dag-isel)
tablegen(LLVM RISCVGenDisassemblerTables.inc -gen-disassembler
- --specialize-decoders-per-bitwidth)
+ --specialize-decoders-per-bitwidth
+ -ignore-non-decodable-operands)
tablegen(LLVM RISCVGenInstrInfo.inc -gen-instr-info)
tablegen(LLVM RISCVGenMCCodeEmitter.inc -gen-emitter)
tablegen(LLVM RISCVGenMCPseudoLowering.inc -gen-pseudo-lowering)
diff --git a/llvm/test/TableGen/AsmPredicateCombining.td b/llvm/test/TableGen/AsmPredicateCombining.td
index f7c0ae7c71110..c8081a428d7bb 100644
--- a/llvm/test/TableGen/AsmPredicateCombining.td
+++ b/llvm/test/TableGen/AsmPredicateCombining.td
@@ -1,4 +1,4 @@
-// RUN: llvm-tblgen -gen-disassembler -I %p/../../include %s | \
+// RUN: llvm-tblgen -gen-disassembler -ignore-non-decodable-operands -I %p/../../include %s | \
// RUN: FileCheck --check-prefix=DISASS %s
// RUN: llvm-tblgen -gen-asm-matcher -I %p/../../include %s | \
// RUN: FileCheck --check-prefix=MATCHER %s
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 339063dd112a6..ebb7deb757c44 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -104,6 +104,18 @@ static cl::opt<bool> SpecializeDecodersPerBitwidth(
"Helps reduce the code size."),
cl::init(false), cl::cat(DisassemblerEmitterCat));
+static cl::opt<bool> IgnoreNonDecodableOperands(
+ "ignore-non-decodable-operands",
+ cl::desc(
+ "Do not issue an error if an operand cannot be decoded automatically."),
+ cl::init(false), cl::cat(DisassemblerEmitterCat));
+
+static cl::opt<bool> IgnoreFullyDefinedOperands(
+ "ignore-fully-defined-operands",
+ cl::desc(
+ "Do not automatically decode operands with no '?' in their encoding."),
+ cl::init(false), cl::cat(DisassemblerEmitterCat));
+
STATISTIC(NumEncodings, "Number of encodings considered");
STATISTIC(NumEncodingsLackingDisasm,
"Number of encodings without disassembler info");
@@ -149,7 +161,7 @@ struct OperandInfo {
std::vector<EncodingField> Fields;
std::string Decoder;
bool HasCompleteDecoder;
- uint64_t InitValue = 0;
+ std::optional<uint64_t> InitValue;
OperandInfo(std::string D, bool HCD) : Decoder(D), HasCompleteDecoder(HCD) {}
@@ -1110,11 +1122,29 @@ FilterChooser::getIslands(const KnownBits &EncodingBits) const {
void DecoderTableBuilder::emitBinaryParser(raw_ostream &OS, indent Indent,
const OperandInfo &OpInfo) const {
- bool UseInsertBits = OpInfo.numFields() != 1 || OpInfo.InitValue != 0;
+ // Special case for 'bits<0>'.
+ if (OpInfo.Fields.empty() && !OpInfo.InitValue) {
+ if (IgnoreNonDecodableOperands)
+ return;
+ assert(!OpInfo.Decoder.empty());
+ // The operand has no encoding, so the corresponding argument is omitted.
+ // This avoids confusion and allows the function to be overloaded if the
+ // operand does have an encoding in other instructions.
+ OS << Indent << "if (!Check(S, " << OpInfo.Decoder << "(MI, Decoder)))\n"
+ << Indent << " return MCDisassembler::Fail;\n";
+ return;
+ }
+
+ if (OpInfo.Fields.empty() && OpInfo.InitValue && IgnoreFullyDefinedOperands)
+ return;
+
+ // We need to construct the encoding of the operand from pieces if it is not
+ // encoded sequentially or has a non-zero constant part in the encoding.
+ bool UseInsertBits = OpInfo.numFields() > 1 || OpInfo.InitValue.value_or(0);
if (UseInsertBits) {
OS << Indent << "tmp = 0x";
- OS.write_hex(OpInfo.InitValue);
+ OS.write_hex(OpInfo.InitValue.value_or(0));
OS << ";\n";
}
@@ -1158,8 +1188,7 @@ void DecoderTableBuilder::emitDecoder(raw_ostream &OS, indent Indent,
}
for (const OperandInfo &Op : Encoding.getOperands())
- if (Op.numFields())
- emitBinaryParser(OS, Indent, Op);
+ emitBinaryParser(OS, Indent, Op);
}
unsigned DecoderTableBuilder::getDecoderIndex(unsigned EncodingID) const {
@@ -1941,15 +1970,48 @@ static void debugDumpRecord(const Record &Rec) {
static void addOneOperandFields(const Record *EncodingDef,
const BitsInit &InstBits,
std::map<StringRef, StringRef> &TiedNames,
- StringRef OpName, OperandInfo &OpInfo) {
- // Some bits of the operand may be required to be 1 depending on the
- // instruction's encoding. Collect those bits.
- if (const RecordVal *EncodedValue = EncodingDef->getValue(OpName))
- if (const BitsInit *OpBits = dyn_cast<BitsInit>(EncodedValue->getValue()))
- for (unsigned I = 0; I < OpBits->getNumBits(); ++I)
- if (const BitInit *OpBit = dyn_cast<BitInit>(OpBits->getBit(I)))
- if (OpBit->getValue())
- OpInfo.InitValue |= 1ULL << I;
+ const Record *OpRec, StringRef OpName,
+ OperandInfo &OpInfo) {
+ // Find a field with the operand's name.
+ const RecordVal *OpEncodingField = EncodingDef->getValue(OpName);
+
+ // If there is no such field, try tied operand's name.
+ if (!OpEncodingField) {
+ if (auto I = TiedNames.find(OpName); I != TiedNames.end())
+ OpEncodingField = EncodingDef->getValue(I->second);
+
+ // If still no luck, the old behavior is to not decode this operand
+ // automatically and let the target do it. This is error-prone, so
+ // the new behavior is to report an error.
+ if (!OpEncodingField) {
+ if (!IgnoreNonDecodableOperands)
+ PrintError(EncodingDef->getLoc(),
+ "could not find field for operand '" + OpName + "'");
+ return;
+ }
+ }
+
+ // Some or all bits of the operand may be required to be 0 or 1 depending
+ // on the instruction's encoding. Collect those bits.
+ if (const auto *OpBit = dyn_cast<BitInit>(OpEncodingField->getValue())) {
+ OpInfo.InitValue = OpBit->getValue();
+ return;
+ }
+ if (const auto *OpBits = dyn_cast<BitsInit>(OpEncodingField->getValue())) {
+ if (OpBits->getNumBits() == 0) {
+ if (OpInfo.Decoder.empty()) {
+ PrintError(EncodingDef->getLoc(), "operand '" + OpName + "' of type '" +
+ OpRec->getName() +
+ "' must have a decoder method");
+ }
+ return;
+ }
+ for (unsigned I = 0; I < OpBits->getNumBits(); ++I) {
+ if (const auto *OpBit = dyn_cast<BitInit>(OpBits->getBit(I)))
+ OpInfo.InitValue = OpInfo.InitValue.value_or(0) |
+ static_cast<uint64_t>(OpBit->getValue()) << I;
+ }
+ }
// Find out where the variable bits of the operand are encoded. The bits don't
// have to be consecutive or in ascending order. For example, an operand could
@@ -2034,8 +2096,10 @@ void InstructionEncoding::parseFixedLenOperands(const BitsInit &Bits) {
// Decode each of the sub-ops separately.
for (auto [SubOpName, SubOp] :
zip_equal(Op.SubOpNames, Op.MIOperandInfo->getArgs())) {
- OperandInfo SubOpInfo = getOpInfo(cast<DefInit>(SubOp)->getDef());
- addOneOperandFields(EncodingDef, Bits, TiedNames, SubOpName, SubOpInfo);
+ const Record *SubOpRec = cast<DefInit>(SubOp)->getDef();
+ OperandInfo SubOpInfo = getOpInfo(SubOpRec);
+ addOneOperandFields(EncodingDef, Bits, TiedNames, SubOpRec, SubOpName,
+ SubOpInfo);
Operands.push_back(std::move(SubOpInfo));
}
continue;
@@ -2056,13 +2120,8 @@ void InstructionEncoding::parseFixedLenOperands(const BitsInit &Bits) {
}
}
- addOneOperandFields(EncodingDef, Bits, TiedNames, Op.Name, OpInfo);
- // FIXME: it should be an error not to find a definition for a given
- // operand, rather than just failing to add it to the resulting
- // instruction! (This is a longstanding bug, which will be addressed in an
- // upcoming change.)
- if (OpInfo.numFields() > 0)
- Operands.push_back(std::move(OpInfo));
+ addOneOperandFields(EncodingDef, Bits, TiedNames, Op.Rec, Op.Name, OpInfo);
+ Operands.push_back(std::move(OpInfo));
}
}
More information about the llvm-commits
mailing list