[llvm] [TableGen][DecoderEmitter] Avoid using a sentinel value (PR #153986)
Sergei Barannikov via llvm-commits
llvm-commits at lists.llvm.org
Sat Aug 16 19:20:32 PDT 2025
https://github.com/s-barannikov updated https://github.com/llvm/llvm-project/pull/153986
>From f90dfef4d486656f1e291f654320e9a5e83965b2 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Sun, 17 Aug 2025 03:29:00 +0300
Subject: [PATCH 1/2] [TableGen][DecoderEmitter] Avoid using a sentinel value
`NO_FIXED_SEGMENTS_SENTINEL` has a value that is actually a valid field
encoding and so it cannot be used as a sentinel.
Replace the sentinel with a new member variable, `VariableFC`, that
contains the value previously stored in `FilterChooserMap` with
`NO_FIXED_SEGMENTS_SENTINEL` key.
---
llvm/utils/TableGen/DecoderEmitter.cpp | 87 +++++++++++++-------------
1 file changed, 42 insertions(+), 45 deletions(-)
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 630793e027116..329d04f9c45ee 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -350,9 +350,6 @@ typedef std::vector<BitValue> insn_t;
namespace {
-static constexpr uint64_t NO_FIXED_SEGMENTS_SENTINEL =
- std::numeric_limits<uint64_t>::max();
-
class FilterChooser;
/// Filter - Filter works with FilterChooser to produce the decoding tree for
@@ -406,6 +403,9 @@ class Filter {
// Map of well-known segment value to its delegate.
std::map<uint64_t, std::unique_ptr<const FilterChooser>> FilterChooserMap;
+ // A filter chooser for encodings that contain some '?' in the filtered range.
+ std::unique_ptr<const FilterChooser> VariableFC;
+
// Number of instructions which fall under FilteredInstructions category.
unsigned NumFiltered;
@@ -425,8 +425,8 @@ class Filter {
// Return the filter chooser for the group of instructions without constant
// segment values.
const FilterChooser &getVariableFC() const {
- assert(NumFiltered == 1 && FilterChooserMap.size() == 1);
- return *(FilterChooserMap.find(NO_FIXED_SEGMENTS_SENTINEL)->second);
+ assert(NumFiltered == 1 && FilterChooserMap.empty());
+ return *VariableFC;
}
// Divides the decoding task into sub tasks and delegates them to the
@@ -658,7 +658,7 @@ Filter::Filter(Filter &&f)
FilteredInstructions(std::move(f.FilteredInstructions)),
VariableInstructions(std::move(f.VariableInstructions)),
FilterChooserMap(std::move(f.FilterChooserMap)),
- NumFiltered(f.NumFiltered) {}
+ VariableFC(std::move(f.VariableFC)), NumFiltered(f.NumFiltered) {}
Filter::Filter(const FilterChooser &owner, unsigned startBit, unsigned numBits)
: Owner(owner), StartBit(startBit), NumBits(numBits) {
@@ -706,17 +706,15 @@ void Filter::recurse() {
// Delegates to an inferior filter chooser for further processing on this
// group of instructions whose segment values are variable.
- FilterChooserMap.try_emplace(
- NO_FIXED_SEGMENTS_SENTINEL,
- std::make_unique<FilterChooser>(Owner.AllInstructions,
- VariableInstructions, Owner.Operands,
- BitValueArray, Owner));
+ VariableFC = std::make_unique<FilterChooser>(
+ Owner.AllInstructions, VariableInstructions, Owner.Operands,
+ BitValueArray, Owner);
}
// No need to recurse for a singleton filtered instruction.
// See also Filter::emit*().
if (getNumFiltered() == 1) {
- assert(FilterChooserMap.size() == 1);
+ assert(VariableFC && "Shouldn't have created a filter for one encoding!");
return;
}
@@ -746,46 +744,30 @@ void Filter::emitTableEntry(DecoderTableInfo &TableInfo) const {
TableInfo.Table.insertULEB128(StartBit);
TableInfo.Table.push_back(NumBits);
- // If the NO_FIXED_SEGMENTS_SENTINEL is present, we need to add a new scope
- // for this filter. Otherwise, we can skip adding a new scope and any
- // patching added will automatically be added to the enclosing scope.
-
- // If NO_FIXED_SEGMENTS_SENTINEL is present, it will be last entry in
- // FilterChooserMap.
-
+ // If VariableFC is present, we need to add a new scope for this filter.
+ // Otherwise, we can skip adding a new scope and any patching added will
+ // automatically be added to the enclosing scope.
const uint64_t LastFilter = FilterChooserMap.rbegin()->first;
- bool HasFallthrough = LastFilter == NO_FIXED_SEGMENTS_SENTINEL;
- if (HasFallthrough)
- TableInfo.pushScope();
+ if (VariableFC)
+ TableInfo.FixupStack.emplace_back();
DecoderTable &Table = TableInfo.Table;
size_t PrevFilter = 0;
for (const auto &[FilterVal, Delegate] : FilterChooserMap) {
- // Field value NO_FIXED_SEGMENTS_SENTINEL implies a non-empty set of
- // variable instructions. See also recurse().
- if (FilterVal == NO_FIXED_SEGMENTS_SENTINEL) {
- // Each scope should always have at least one filter value to check
- // for.
- assert(PrevFilter != 0 && "empty filter set!");
- TableInfo.popScope();
- PrevFilter = 0; // Don't re-process the filter's fallthrough.
+ // The last filtervalue emitted can be OPC_FilterValue if we are at
+ // outermost scope.
+ const uint8_t DecoderOp =
+ FilterVal == LastFilter && TableInfo.isOutermostScope()
+ ? MCD::OPC_FilterValueOrFail
+ : MCD::OPC_FilterValue;
+ Table.push_back(DecoderOp);
+ Table.insertULEB128(FilterVal);
+ if (DecoderOp == MCD::OPC_FilterValue) {
+ // Reserve space for the NumToSkip entry. We'll backpatch the value later.
+ PrevFilter = Table.insertNumToSkip();
} else {
- // The last filtervalue emitted can be OPC_FilterValue if we are at
- // outermost scope.
- const uint8_t DecoderOp =
- FilterVal == LastFilter && TableInfo.isOutermostScope()
- ? MCD::OPC_FilterValueOrFail
- : MCD::OPC_FilterValue;
- Table.push_back(DecoderOp);
- Table.insertULEB128(FilterVal);
- if (DecoderOp == MCD::OPC_FilterValue) {
- // Reserve space for the NumToSkip entry. We'll backpatch the value
- // later.
- PrevFilter = Table.insertNumToSkip();
- } else {
- PrevFilter = 0;
- }
+ PrevFilter = 0;
}
// We arrive at a category of instructions with the same segment value.
@@ -800,6 +782,21 @@ void Filter::emitTableEntry(DecoderTableInfo &TableInfo) const {
Table.patchNumToSkip(PrevFilter, Table.size());
}
+ if (VariableFC) {
+ // Each scope should always have at least one filter value to check for.
+ assert(PrevFilter != 0 && "empty filter set!");
+ // Resolve any NumToSkip fixups in the current scope.
+ resolveTableFixups(Table, TableInfo.FixupStack.back(), Table.size());
+
+ // Delete the scope we have added here.
+ TableInfo.FixupStack.pop_back();
+
+ PrevFilter = 0; // Don't re-process the filter's fallthrough.
+
+ // Delegate to the sub filter chooser for further decoding.
+ VariableFC->emitTableEntries(TableInfo);
+ }
+
// If there is no fallthrough and the final filter was not in the outermost
// scope, then it must be fixed up according to the enclosing scope rather
// than the current position.
>From 94e24fc0f3a1a61c7cbfb962845c84fa46218634 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Sun, 17 Aug 2025 04:50:35 +0300
Subject: [PATCH 2/2] Add a test
---
.../FixedLenDecoderEmitter/big-filter.td | 40 +++++++++++++++++++
llvm/utils/TableGen/DecoderEmitter.cpp | 7 +---
2 files changed, 41 insertions(+), 6 deletions(-)
create mode 100644 llvm/test/TableGen/FixedLenDecoderEmitter/big-filter.td
diff --git a/llvm/test/TableGen/FixedLenDecoderEmitter/big-filter.td b/llvm/test/TableGen/FixedLenDecoderEmitter/big-filter.td
new file mode 100644
index 0000000000000..b9da61de469ac
--- /dev/null
+++ b/llvm/test/TableGen/FixedLenDecoderEmitter/big-filter.td
@@ -0,0 +1,40 @@
+// RUN: llvm-tblgen -gen-disassembler -I %p/../../../include %s | FileCheck %s
+
+include "llvm/Target/Target.td"
+
+class I : Instruction {
+ let InOperandList = (ins);
+ let OutOperandList = (outs);
+ let Size = 16;
+ bits<128> Inst;
+}
+
+// Check that a 64-bit filter with all bits set does not confuse DecoderEmitter.
+//
+// CHECK-LABEL: static const uint8_t DecoderTable128[] = {
+// CHECK-NEXT: MCD::OPC_ExtractField, 0, 64,
+// CHECK-NEXT: MCD::OPC_FilterValue, 1, 8, 0,
+// CHECK-NEXT: MCD::OPC_CheckFieldOrFail, 127, 1, 1,
+// CHECK-NEXT: MCD::OPC_Decode, 187, 2, 0,
+// CHECK-NEXT: MCD::OPC_FilterValueOrFail, 255, 255, 255, 255, 255, 255, 255, 255, 255, 1,
+// CHECK-NEXT: MCD::OPC_CheckFieldOrFail, 127, 1, 0,
+// CHECK-NEXT: MCD::OPC_Decode, 186, 2, 0,
+// CHECK-NEXT: MCD::OPC_Fail,
+// CHECK-NEXT: 0
+// CHECK-NEXT: };
+
+def I1 : I {
+ let Inst{63...0} = -1;
+ let Inst{127} = 0;
+}
+
+def I2 : I {
+ let Inst{63...0} = 1;
+ let Inst{127} = 1;
+}
+
+def II : InstrInfo;
+
+def MyTarget : Target {
+ let InstructionSet = II;
+}
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 329d04f9c45ee..daf91b376ade2 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -785,12 +785,7 @@ void Filter::emitTableEntry(DecoderTableInfo &TableInfo) const {
if (VariableFC) {
// Each scope should always have at least one filter value to check for.
assert(PrevFilter != 0 && "empty filter set!");
- // Resolve any NumToSkip fixups in the current scope.
- resolveTableFixups(Table, TableInfo.FixupStack.back(), Table.size());
-
- // Delete the scope we have added here.
- TableInfo.FixupStack.pop_back();
-
+ TableInfo.popScope();
PrevFilter = 0; // Don't re-process the filter's fallthrough.
// Delegate to the sub filter chooser for further decoding.
More information about the llvm-commits
mailing list