[TableGen][DecoderEmitter] Infer encoding's HasCompleteDecoder earlier (NFCI) #154644
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-tablegen Author: Sergei Barannikov (s-barannikov) ChangesIf an encoding has a custom decoder, the decoder is assumed to be "complete" (always succeed) if hasCompleteDecoder field is true. We determine this when constructing InstructionEncoding. If the decoder for an encoding is generated, it always succeeds if none of the operand decoders can fail. The latter is determined based on the value of operands' DecoderMethod/hasCompleteDecoder. This happens late, at emission time, making the code harder to follow. This change moves this logic to the InstructionEncoding constructor. Full diff: https://github.com/llvm/llvm-project/pull/154644.diff 1 Files Affected:
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 8fa44bcc7b964..62903a10b26b2 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -147,9 +147,10 @@ class InstructionEncoding {
/// meaning the decoder is generated.
StringRef DecoderMethod;
- /// Whether the custom decoding function always succeeds. Should not be used
- /// if the decoder is generated.
- bool HasCompleteDecoder = true;
+ /// Whether the custom decoding function always succeeds. If a custom decoder
+ /// function is specified, the value is taken from the target description,
+ /// otherwise it is inferred.
+ bool HasCompleteDecoder;
/// Information about the operands' contribution to this encoding.
SmallVector<OperandInfo, 16> Operands;
@@ -169,6 +170,16 @@ class InstructionEncoding {
HasCompleteDecoder = EncodingDef->getValueAsBit("hasCompleteDecoder");
populateEncoding();
+
+ if (DecoderMethod.empty()) {
+ // A generated decoder is always successful if none of the operand
+ // decoders can fail (all are always successful).
+ HasCompleteDecoder = all_of(Operands, [](const OperandInfo &Op) {
+ // By default, a generated operand decoder is assumed to always succeed.
+ // This can be overridden by the user.
+ return Op.Decoder.empty() || Op.HasCompleteDecoder;
+ });
+ }
}
/// Returns the Record this encoding originates from.
@@ -187,11 +198,9 @@ class InstructionEncoding {
/// if the decoder is generated.
StringRef getDecoderMethod() const { return DecoderMethod; }
- /// Returns whether the custom decoding function always succeeds.
- bool hasCompleteDecoder() const {
- assert(!DecoderMethod.empty());
- return HasCompleteDecoder;
- }
+ /// Returns whether the decoder (either generated or specified by the user)
+ /// always succeeds.
+ bool hasCompleteDecoder() const { return HasCompleteDecoder; }
/// Returns information about the operands' contribution to this encoding.
ArrayRef<OperandInfo> getOperands() const { return Operands; }
@@ -672,12 +681,11 @@ class FilterChooser {
void emitSingletonTableEntry(DecoderTableInfo &TableInfo,
const Filter &Best) const;
- bool emitBinaryParser(raw_ostream &OS, indent Indent,
+ void emitBinaryParser(raw_ostream &OS, indent Indent,
const OperandInfo &OpInfo) const;
- bool emitDecoder(raw_ostream &OS, indent Indent, unsigned EncodingID) const;
- std::pair<unsigned, bool> getDecoderIndex(DecoderSet &Decoders,
- unsigned EncodingID) const;
+ void emitDecoder(raw_ostream &OS, indent Indent, unsigned EncodingID) const;
+ unsigned getDecoderIndex(DecoderSet &Decoders, unsigned EncodingID) const;
// Assign a single filter and run with it.
void runSingleFilter(unsigned startBit, unsigned numBit);
@@ -1226,7 +1234,7 @@ FilterChooser::getIslands(const insn_t &Insn) const {
return Islands;
}
-bool FilterChooser::emitBinaryParser(raw_ostream &OS, indent Indent,
+void FilterChooser::emitBinaryParser(raw_ostream &OS, indent Indent,
const OperandInfo &OpInfo) const {
const std::string &Decoder = OpInfo.Decoder;
@@ -1252,65 +1260,56 @@ bool FilterChooser::emitBinaryParser(raw_ostream &OS, indent Indent,
OS << ";\n";
}
- bool OpHasCompleteDecoder;
if (!Decoder.empty()) {
- OpHasCompleteDecoder = OpInfo.HasCompleteDecoder;
OS << Indent << "if (!Check(S, " << Decoder
<< "(MI, tmp, Address, Decoder))) { "
- << (OpHasCompleteDecoder ? "" : "DecodeComplete = false; ")
+ << (OpInfo.HasCompleteDecoder ? "" : "DecodeComplete = false; ")
<< "return MCDisassembler::Fail; }\n";
} else {
- OpHasCompleteDecoder = true;
OS << Indent << "MI.addOperand(MCOperand::createImm(tmp));\n";
}
- return OpHasCompleteDecoder;
}
-bool FilterChooser::emitDecoder(raw_ostream &OS, indent Indent,
+void FilterChooser::emitDecoder(raw_ostream &OS, indent Indent,
unsigned EncodingID) const {
const InstructionEncoding &Encoding = Encodings[EncodingID];
// If a custom instruction decoder was specified, use that.
StringRef DecoderMethod = Encoding.getDecoderMethod();
if (!DecoderMethod.empty()) {
- bool HasCompleteDecoder = Encoding.hasCompleteDecoder();
OS << Indent << "if (!Check(S, " << DecoderMethod
<< "(MI, insn, Address, Decoder))) { "
- << (HasCompleteDecoder ? "" : "DecodeComplete = false; ")
+ << (Encoding.hasCompleteDecoder() ? "" : "DecodeComplete = false; ")
<< "return MCDisassembler::Fail; }\n";
- return HasCompleteDecoder;
+ return;
}
- bool HasCompleteDecoder = true;
for (const OperandInfo &Op : Encoding.getOperands()) {
// FIXME: This is broken. If there is an operand that doesn't contribute
// to the encoding, we generate the same code as if the decoder method
// was specified on the encoding. And then we stop, ignoring the rest
// of the operands. M68k disassembler experiences this.
if (Op.numFields() == 0 && !Op.Decoder.empty()) {
- HasCompleteDecoder = Op.HasCompleteDecoder;
OS << Indent << "if (!Check(S, " << Op.Decoder
<< "(MI, insn, Address, Decoder))) { "
- << (HasCompleteDecoder ? "" : "DecodeComplete = false; ")
+ << (Op.HasCompleteDecoder ? "" : "DecodeComplete = false; ")
<< "return MCDisassembler::Fail; }\n";
break;
}
- HasCompleteDecoder &= emitBinaryParser(OS, Indent, Op);
+ emitBinaryParser(OS, Indent, Op);
}
- return HasCompleteDecoder;
}
-std::pair<unsigned, bool>
-FilterChooser::getDecoderIndex(DecoderSet &Decoders,
- unsigned EncodingID) const {
+unsigned FilterChooser::getDecoderIndex(DecoderSet &Decoders,
+ unsigned EncodingID) const {
// Build up the predicate string.
SmallString<256> Decoder;
// FIXME: emitDecoder() function can take a buffer directly rather than
// a stream.
raw_svector_ostream S(Decoder);
indent Indent(UseFnTableInDecodeToMCInst ? 2 : 4);
- bool HasCompleteDecoder = emitDecoder(S, Indent, EncodingID);
+ emitDecoder(S, Indent, EncodingID);
// Using the full decoder string as the key value here is a bit
// heavyweight, but is effective. If the string comparisons become a
@@ -1322,7 +1321,7 @@ FilterChooser::getDecoderIndex(DecoderSet &Decoders,
Decoders.insert(CachedHashString(Decoder));
// Now figure out the index for when we write out the table.
DecoderSet::const_iterator P = find(Decoders, Decoder.str());
- return {(unsigned)(P - Decoders.begin()), HasCompleteDecoder};
+ return std::distance(Decoders.begin(), P);
}
// If ParenIfBinOp is true, print a surrounding () if Val uses && or ||.
@@ -1522,8 +1521,7 @@ void FilterChooser::emitSingletonTableEntry(DecoderTableInfo &TableInfo,
// Check for soft failure of the match.
emitSoftFailTableEntry(TableInfo, EncodingID);
- auto [DIdx, HasCompleteDecoder] =
- getDecoderIndex(TableInfo.Decoders, EncodingID);
+ unsigned DIdx = getDecoderIndex(TableInfo.Decoders, EncodingID);
// Produce OPC_Decode or OPC_TryDecode opcode based on the information
// whether the instruction decoder is complete or not. If it is complete
@@ -1534,10 +1532,12 @@ void FilterChooser::emitSingletonTableEntry(DecoderTableInfo &TableInfo,
// decoder method indicates that additional processing should be done to see
// if there is any other instruction that also matches the bitpattern and
// can decode it.
- const uint8_t DecoderOp = HasCompleteDecoder ? MCD::OPC_Decode
- : (TableInfo.isOutermostScope()
- ? MCD::OPC_TryDecodeOrFail
- : MCD::OPC_TryDecode);
+ const InstructionEncoding &Encoding = Encodings[EncodingID];
+ const uint8_t DecoderOp =
+ Encoding.hasCompleteDecoder()
+ ? MCD::OPC_Decode
+ : (TableInfo.isOutermostScope() ? MCD::OPC_TryDecodeOrFail
+ : MCD::OPC_TryDecode);
TableInfo.Table.push_back(DecoderOp);
const Record *InstDef = Encodings[EncodingID].getInstruction()->TheDef;
TableInfo.Table.insertULEB128(Emitter->getTarget().getInstrIntValue(InstDef));
|
s-barannikov
force-pushed
the
tablegen/decoder/operands-complete-decoder
branch
from
494dffb to
23d55e1
Compare
jurahul
approved these changes
Aug 21, 2025
…r (NFCI) If an encoding has a custom decoder, the decoder is assumed to be "complete" (always succeed) if hasCompleteDecoder field is true. We determine this when constructing InstructionEncoding. If the decoder for an encoding is *generated*, it always succeeds if none of the operand decoders can fail. The latter is determined based on the value of operands' DecoderMethod/hasCompleteDecoder. This happens late, at emission time, complicating the code. This change moves this logic to the InstructionEncoding constructor.
s-barannikov
force-pushed
the
tablegen/decoder/operands-complete-decoder
branch
from
23d55e1 to
5794f99
Compare
Contributor
Author
|
Rebased to resolve conflicts |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/29483 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If an encoding has a custom decoder, the decoder is assumed to be "complete" (always succeed) if hasCompleteDecoder field is true. We determine this when constructing InstructionEncoding.
If the decoder for an encoding is generated, it always succeeds if none of the operand decoders can fail. The latter is determined based on the value of operands' DecoderMethod/hasCompleteDecoder. This happens late, at table construction time, making the code harder to follow.
This change moves this logic to the InstructionEncoding constructor.