[llvm] 11c6158 - [NFCI][MC][DecoderEmitter] Fix BitWidth for fixed length inst encodings (#154934)
via llvm-commits
llvm-commits at lists.llvm.org
Sun Aug 24 07:04:11 PDT 2025
Author: Rahul Joshi
Date: 2025-08-24T07:04:07-07:00
New Revision: 11c615818feaa705cbbe7083f40d6c6a5641b704
URL: https://github.com/llvm/llvm-project/commit/11c615818feaa705cbbe7083f40d6c6a5641b704
DIFF: https://github.com/llvm/llvm-project/commit/11c615818feaa705cbbe7083f40d6c6a5641b704.diff
LOG: [NFCI][MC][DecoderEmitter] Fix BitWidth for fixed length inst encodings (#154934)
Change `InstructionEncoding` to use `Size` field to derive the BitWidth
for fixed length instructions as opposed to the number of bits in the
`Inst` field. For some backends, `Inst` has more bits than `Size`, but
`Size` is the true size of the instruction.
Also add validation that `Inst` has at least `Size * 8` bits and any
bits in `Inst` beyond that are either 0 or unset.
Verified no change in generated *GenDisassembler.inc files before/after.
Added:
llvm/test/TableGen/FixedLenDecoderEmitter/InvalidEncoding.td
Modified:
llvm/utils/TableGen/DecoderEmitter.cpp
Removed:
################################################################################
diff --git a/llvm/test/TableGen/FixedLenDecoderEmitter/InvalidEncoding.td b/llvm/test/TableGen/FixedLenDecoderEmitter/InvalidEncoding.td
new file mode 100644
index 0000000000000..074c7c53e85c4
--- /dev/null
+++ b/llvm/test/TableGen/FixedLenDecoderEmitter/InvalidEncoding.td
@@ -0,0 +1,53 @@
+// RUN: not llvm-tblgen -gen-disassembler -I %p/../../../include %s -DTEST1 2>&1 | FileCheck %s --check-prefix=CHECK-TEST1
+// RUN: not llvm-tblgen -gen-disassembler -I %p/../../../include %s -DTEST2 2>&1 | FileCheck %s --check-prefix=CHECK-TEST2
+// RUN: not llvm-tblgen -gen-disassembler -I %p/../../../include %s -DTEST3 2>&1 | FileCheck %s --check-prefix=CHECK-TEST3
+
+include "llvm/Target/Target.td"
+
+def archInstrInfo : InstrInfo { }
+
+def arch : Target {
+ let InstructionSet = archInstrInfo;
+}
+
+#ifdef TEST1
+// CHECK-TEST1: [[#@LINE+1]]:5: error: foo: Size is 16 bits, but Inst bits beyond that are not zero/unset
+def foo : Instruction {
+ let OutOperandList = (outs);
+ let InOperandList = (ins i32imm:$factor);
+ let Size = 2;
+ field bits<24> Inst;
+ field bits<24> SoftFail = 0;
+ bits<8> factor;
+ let Inst{15...8} = factor{7...0};
+ let Inst{20} = 1;
+}
+#endif
+
+#ifdef TEST2
+// CHECK-TEST2: [[#@LINE+1]]:5: error: foo: Size is 16 bits, but SoftFail bits beyond that are not zero/unset
+def foo : Instruction {
+ let OutOperandList = (outs);
+ let InOperandList = (ins i32imm:$factor);
+ let Size = 2;
+ field bits<24> Inst;
+ field bits<24> SoftFail = 0;
+ bits<8> factor;
+ let Inst{15...8} = factor{7...0};
+ let Inst{23...16} = 0;
+ let SoftFail{20} = 1;
+}
+#endif
+
+#ifdef TEST3
+// CHECK-TEST3: [[#@LINE+1]]:5: error: bar: Size is 16 bits, but Inst specifies only 8 bits
+def bar : Instruction {
+ let OutOperandList = (outs);
+ let InOperandList = (ins i32imm:$factor);
+ let Size = 2;
+ field bits<8> Inst;
+ field bits<8> SoftFail = 0;
+ bits<4> factor;
+ let Inst{4...1} = factor{3...0};
+}
+#endif
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 758cb8f6f808e..4f6eeb3922f9a 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -223,7 +223,7 @@ class InstructionEncoding {
private:
void parseVarLenEncoding(const VarLenInst &VLI);
- void parseFixedLenEncoding(const BitsInit &Bits);
+ void parseFixedLenEncoding(const BitsInit &RecordInstBits);
void parseVarLenOperands(const VarLenInst &VLI);
void parseFixedLenOperands(const BitsInit &Bits);
@@ -1772,12 +1772,51 @@ void InstructionEncoding::parseVarLenEncoding(const VarLenInst &VLI) {
assert(I == VLI.size());
}
-void InstructionEncoding::parseFixedLenEncoding(const BitsInit &Bits) {
- InstBits = KnownBits(Bits.getNumBits());
- SoftFailBits = APInt(Bits.getNumBits(), 0);
+void InstructionEncoding::parseFixedLenEncoding(
+ const BitsInit &RecordInstBits) {
+ // For fixed length instructions, sometimes the `Inst` field specifies more
+ // bits than the actual size of the instruction, which is specified in `Size`.
+ // In such cases, we do some basic validation and drop the upper bits.
+ unsigned BitWidth = EncodingDef->getValueAsInt("Size") * 8;
+ unsigned InstNumBits = RecordInstBits.getNumBits();
+
+ // Returns true if all bits in `Bits` are zero or unset.
+ auto CheckAllZeroOrUnset = [&](ArrayRef<const Init *> Bits,
+ const RecordVal *Field) {
+ bool AllZeroOrUnset = llvm::all_of(Bits, [](const Init *Bit) {
+ if (const auto *BI = dyn_cast<BitInit>(Bit))
+ return !BI->getValue();
+ return isa<UnsetInit>(Bit);
+ });
+ if (AllZeroOrUnset)
+ return;
+ PrintNote([Field](raw_ostream &OS) { Field->print(OS); });
+ PrintFatalError(EncodingDef, Twine(Name) + ": Size is " + Twine(BitWidth) +
+ " bits, but " + Field->getName() +
+ " bits beyond that are not zero/unset");
+ };
+
+ if (InstNumBits < BitWidth)
+ PrintFatalError(EncodingDef, Twine(Name) + ": Size is " + Twine(BitWidth) +
+ " bits, but Inst specifies only " +
+ Twine(InstNumBits) + " bits");
+
+ if (InstNumBits > BitWidth) {
+ // Ensure that all the bits beyond 'Size' are 0 or unset (i.e., carry no
+ // actual encoding).
+ ArrayRef<const Init *> UpperBits =
+ RecordInstBits.getBits().drop_front(BitWidth);
+ const RecordVal *InstField = EncodingDef->getValue("Inst");
+ CheckAllZeroOrUnset(UpperBits, InstField);
+ }
+
+ ArrayRef<const Init *> ActiveInstBits =
+ RecordInstBits.getBits().take_front(BitWidth);
+ InstBits = KnownBits(BitWidth);
+ SoftFailBits = APInt(BitWidth, 0);
// Parse Inst field.
- for (auto [I, V] : enumerate(Bits.getBits())) {
+ for (auto [I, V] : enumerate(ActiveInstBits)) {
if (const auto *B = dyn_cast<BitInit>(V)) {
if (B->getValue())
InstBits.One.setBit(I);
@@ -1787,26 +1826,36 @@ void InstructionEncoding::parseFixedLenEncoding(const BitsInit &Bits) {
}
// Parse SoftFail field.
- if (const RecordVal *SoftFailField = EncodingDef->getValue("SoftFail")) {
- const auto *SFBits = dyn_cast<BitsInit>(SoftFailField->getValue());
- if (!SFBits || SFBits->getNumBits() != Bits.getNumBits()) {
- PrintNote(EncodingDef->getLoc(), "in record");
- PrintFatalError(SoftFailField,
- formatv("SoftFail field, if defined, must be "
- "of the same type as Inst, which is bits<{}>",
- Bits.getNumBits()));
- }
- for (auto [I, V] : enumerate(SFBits->getBits())) {
- if (const auto *B = dyn_cast<BitInit>(V); B && B->getValue()) {
- if (!InstBits.Zero[I] && !InstBits.One[I]) {
- PrintNote(EncodingDef->getLoc(), "in record");
- PrintError(SoftFailField,
- formatv("SoftFail{{{0}} = 1 requires Inst{{{0}} "
- "to be fully defined (0 or 1, not '?')",
- I));
- }
- SoftFailBits.setBit(I);
+ const RecordVal *SoftFailField = EncodingDef->getValue("SoftFail");
+ if (!SoftFailField)
+ return;
+
+ const auto *SFBits = dyn_cast<BitsInit>(SoftFailField->getValue());
+ if (!SFBits || SFBits->getNumBits() != InstNumBits) {
+ PrintNote(EncodingDef->getLoc(), "in record");
+ PrintFatalError(SoftFailField,
+ formatv("SoftFail field, if defined, must be "
+ "of the same type as Inst, which is bits<{}>",
+ InstNumBits));
+ }
+
+ if (InstNumBits > BitWidth) {
+ // Ensure that all upper bits of `SoftFail` are 0 or unset.
+ ArrayRef<const Init *> UpperBits = SFBits->getBits().drop_front(BitWidth);
+ CheckAllZeroOrUnset(UpperBits, SoftFailField);
+ }
+
+ ArrayRef<const Init *> ActiveSFBits = SFBits->getBits().take_front(BitWidth);
+ for (auto [I, V] : enumerate(ActiveSFBits)) {
+ if (const auto *B = dyn_cast<BitInit>(V); B && B->getValue()) {
+ if (!InstBits.Zero[I] && !InstBits.One[I]) {
+ PrintNote(EncodingDef->getLoc(), "in record");
+ PrintError(SoftFailField,
+ formatv("SoftFail{{{0}} = 1 requires Inst{{{0}} "
+ "to be fully defined (0 or 1, not '?')",
+ I));
}
+ SoftFailBits.setBit(I);
}
}
}
More information about the llvm-commits
mailing list