[TableGen][DecoderEmitter] Merge OPC_Decode with OPC_TryDecode #159178
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
OPC_Decode is a specialized OPC_TryDecode. The difference between them is that OPC_Decode immediately returns from decodeToMCInst, whether the decoding was successful or not, while OPC_TryDecode performs an additional check first. The check is just a boolean test, which is nothing compared to the complexity of the decoding process, so there is no point in having a special opcode that optimizes the check.
|
@llvm/pr-subscribers-tablegen @llvm/pr-subscribers-llvm-mc Author: Sergei Barannikov (s-barannikov) ChangesOPC_Decode is a specialized OPC_TryDecode. The difference between them The check is just a boolean test, which is nothing compared to the Full diff: https://github.com/llvm/llvm-project/pull/159178.diff 7 Files Affected:
diff --git a/llvm/include/llvm/MC/MCDecoderOps.h b/llvm/include/llvm/MC/MCDecoderOps.h
index 790ff3eb4f333..5afc0387f561f 100644
--- a/llvm/include/llvm/MC/MCDecoderOps.h
+++ b/llvm/include/llvm/MC/MCDecoderOps.h
@@ -24,7 +24,6 @@ enum DecoderOps {
// uleb128 Val)
OPC_CheckPredicate, // OPC_CheckPredicate(uleb128 PIdx)
OPC_Decode, // OPC_Decode(uleb128 Opcode, uleb128 DIdx)
- OPC_TryDecode, // OPC_TryDecode(uleb128 Opcode, uleb128 DIdx)
OPC_SoftFail, // OPC_SoftFail(uleb128 PMask, uleb128 NMask)
};
diff --git a/llvm/test/TableGen/DecoderEmitter/additional-encoding.td b/llvm/test/TableGen/DecoderEmitter/additional-encoding.td
index 0d4d3c096f83d..e3ef572bfa7f8 100644
--- a/llvm/test/TableGen/DecoderEmitter/additional-encoding.td
+++ b/llvm/test/TableGen/DecoderEmitter/additional-encoding.td
@@ -35,22 +35,22 @@ class I<dag out_ops, dag in_ops> : Instruction {
// CHECK-NEXT: /* 7 */ OPC_Scope, 8, 0, // end scope at 18
// CHECK-NEXT: /* 10 */ OPC_CheckField, 6, 6, 0, // if Inst{11-6} != 0x0
// CHECK-NEXT: /* 14 */ OPC_Decode, {{[0-9]+}}, 2, 0, // Opcode: {{.*}}:NOP, DecodeIdx: 0
-// CHECK-NEXT: /* 18 */ OPC_TryDecode, {{[0-9]+}}, 2, 1, // Opcode: SHIFT0, DecodeIdx: 1
+// CHECK-NEXT: /* 18 */ OPC_Decode, {{[0-9]+}}, 2, 1, // Opcode: SHIFT0, DecodeIdx: 1
// CHECK-NEXT: /* 22 */ OPC_FilterValueOrSkip, 1, 15, 0, // if Field != 0x1 skip to 41
// CHECK-NEXT: /* 26 */ OPC_Scope, 8, 0, // end scope at 37
// CHECK-NEXT: /* 29 */ OPC_CheckField, 6, 6, 0, // if Inst{11-6} != 0x0
// CHECK-NEXT: /* 33 */ OPC_Decode, {{[0-9]+}}, 2, 0, // Opcode: {{.*}}:NOP, DecodeIdx: 0
-// CHECK-NEXT: /* 37 */ OPC_TryDecode, {{[0-9]+}}, 2, 1, // Opcode: SHIFT1, DecodeIdx: 1
+// CHECK-NEXT: /* 37 */ OPC_Decode, {{[0-9]+}}, 2, 1, // Opcode: SHIFT1, DecodeIdx: 1
// CHECK-NEXT: /* 41 */ OPC_FilterValueOrSkip, 2, 15, 0, // if Field != 0x2 skip to 60
// CHECK-NEXT: /* 45 */ OPC_Scope, 8, 0, // end scope at 56
// CHECK-NEXT: /* 48 */ OPC_CheckField, 6, 6, 0, // if Inst{11-6} != 0x0
// CHECK-NEXT: /* 52 */ OPC_Decode, {{[0-9]+}}, 2, 0, // Opcode: {{.*}}:NOP, DecodeIdx: 0
-// CHECK-NEXT: /* 56 */ OPC_TryDecode, {{[0-9]+}}, 2, 1, // Opcode: SHIFT2, DecodeIdx: 1
+// CHECK-NEXT: /* 56 */ OPC_Decode, {{[0-9]+}}, 2, 1, // Opcode: SHIFT2, DecodeIdx: 1
// CHECK-NEXT: /* 60 */ OPC_FilterValue, 3, // if Field != 0x3
// CHECK-NEXT: /* 62 */ OPC_Scope, 8, 0, // end scope at 73
// CHECK-NEXT: /* 65 */ OPC_CheckField, 6, 6, 0, // if Inst{11-6} != 0x0
// CHECK-NEXT: /* 69 */ OPC_Decode, {{[0-9]+}}, 2, 0, // Opcode: {{.*}}:NOP, DecodeIdx: 0
-// CHECK-NEXT: /* 73 */ OPC_TryDecode, {{[0-9]+}}, 2, 1, // Opcode: SHIFT3, DecodeIdx: 1
+// CHECK-NEXT: /* 73 */ OPC_Decode, {{[0-9]+}}, 2, 1, // Opcode: SHIFT3, DecodeIdx: 1
class SHIFT<bits<2> opc> : I<(outs), (ins ShAmtOp:$shamt)>, EncSHIFT<opc>;
diff --git a/llvm/test/TableGen/DecoderEmitter/trydecode-emission.td b/llvm/test/TableGen/DecoderEmitter/trydecode-emission.td
index cdb1e327ad07d..b04ba2b4a6f5b 100644
--- a/llvm/test/TableGen/DecoderEmitter/trydecode-emission.td
+++ b/llvm/test/TableGen/DecoderEmitter/trydecode-emission.td
@@ -37,7 +37,7 @@ def InstB : TestInstruction {
// CHECK: /* 0 */ OPC_CheckField, 4, 4, 0, // if Inst{7-4} != 0x0
// CHECK-NEXT: /* 4 */ OPC_Scope, 8, 0, // end scope at 15
// CHECK-NEXT: /* 7 */ OPC_CheckField, 2, 2, 0, // if Inst{3-2} != 0x0
-// CHECK-NEXT: /* 11 */ OPC_TryDecode, {{[0-9]+}}, {{[0-9]+}}, 0, // Opcode: InstB, DecodeIdx: 0
+// CHECK-NEXT: /* 11 */ OPC_Decode, {{[0-9]+}}, {{[0-9]+}}, 0, // Opcode: InstB, DecodeIdx: 0
// CHECK-NEXT: /* 15 */ OPC_Decode, {{[0-9]+}}, {{[0-9]+}}, 1, // Opcode: InstA, DecodeIdx: 1
// CHECK-NEXT: };
@@ -50,7 +50,7 @@ def InstB : TestInstruction {
// CHECK-LARGE: /* 0 */ OPC_CheckField, 4, 4, 0, // if Inst{7-4} != 0x0
// CHECK-LARGE-NEXT: /* 4 */ OPC_Scope, 8, 0, 0, // end scope at 16
// CHECK-LARGE-NEXT: /* 8 */ OPC_CheckField, 2, 2, 0, // if Inst{3-2} != 0x0
-// CHECK-LARGE-NEXT: /* 12 */ OPC_TryDecode, {{[0-9]+}}, {{[0-9]+}}, 0, // Opcode: InstB, DecodeIdx: 0
+// CHECK-LARGE-NEXT: /* 12 */ OPC_Decode, {{[0-9]+}}, {{[0-9]+}}, 0, // Opcode: InstB, DecodeIdx: 0
// CHECK-LARGE-NEXT: /* 16 */ OPC_Decode, {{[0-9]+}}, {{[0-9]+}}, 1, // Opcode: InstA, DecodeIdx: 1
// CHECK-LARGE-NEXT: };
diff --git a/llvm/test/TableGen/DecoderEmitter/trydecode-emission2.td b/llvm/test/TableGen/DecoderEmitter/trydecode-emission2.td
index 35657ff35c86f..7fd26fffd28b7 100644
--- a/llvm/test/TableGen/DecoderEmitter/trydecode-emission2.td
+++ b/llvm/test/TableGen/DecoderEmitter/trydecode-emission2.td
@@ -35,9 +35,9 @@ def InstB : TestInstruction {
// CHECK-NEXT: /* 4 */ OPC_CheckField, 5, 3, 0,
// CHECK-NEXT: /* 8 */ OPC_Scope, 8, 0, // end scope at 19
// CHECK-NEXT: /* 11 */ OPC_CheckField, 0, 2, 3,
-// CHECK-NEXT: /* 15 */ OPC_TryDecode, {{[0-9]+}}, {{[0-9]+}}, 0,
+// CHECK-NEXT: /* 15 */ OPC_Decode, {{[0-9]+}}, {{[0-9]+}}, 0,
// CHECK-NEXT: /* 19 */ OPC_CheckField, 3, 2, 0,
-// CHECK-NEXT: /* 23 */ OPC_TryDecode, {{[0-9]+}}, {{[0-9]+}}, 1,
+// CHECK-NEXT: /* 23 */ OPC_Decode, {{[0-9]+}}, {{[0-9]+}}, 1,
// CHECK: if (!Check(S, DecodeInstB(MI, insn, Address, Decoder))) { DecodeComplete = false; return MCDisassembler::Fail; }
// CHECK: if (!Check(S, DecodeInstA(MI, insn, Address, Decoder))) { DecodeComplete = false; return MCDisassembler::Fail; }
@@ -46,9 +46,9 @@ def InstB : TestInstruction {
// CHECK-LARGE-NEXT: /* 4 */ OPC_CheckField, 5, 3, 0,
// CHECK-LARGE-NEXT: /* 8 */ OPC_Scope, 8, 0, 0, // end scope at 20
// CHECK-LARGE-NEXT: /* 12 */ OPC_CheckField, 0, 2, 3,
-// CHECK-LARGE-NEXT: /* 16 */ OPC_TryDecode, {{[0-9]+}}, {{[0-9]+}}, 0,
+// CHECK-LARGE-NEXT: /* 16 */ OPC_Decode, {{[0-9]+}}, {{[0-9]+}}, 0,
// CHECK-LARGE-NEXT: /* 20 */ OPC_CheckField, 3, 2, 0,
-// CHECK-LARGE-NEXT: /* 24 */ OPC_TryDecode, {{[0-9]+}}, {{[0-9]+}}, 1,
+// CHECK-LARGE-NEXT: /* 24 */ OPC_Decode, {{[0-9]+}}, {{[0-9]+}}, 1,
// CHECK-LARGE-NEXT: };
// CHECK-LARGE: if (!Check(S, DecodeInstB(MI, insn, Address, Decoder))) { DecodeComplete = false; return MCDisassembler::Fail; }
diff --git a/llvm/test/TableGen/DecoderEmitter/trydecode-emission3.td b/llvm/test/TableGen/DecoderEmitter/trydecode-emission3.td
index 4ac868dbb51aa..c884d6b8a93cc 100644
--- a/llvm/test/TableGen/DecoderEmitter/trydecode-emission3.td
+++ b/llvm/test/TableGen/DecoderEmitter/trydecode-emission3.td
@@ -38,7 +38,7 @@ def InstB : TestInstruction {
// CHECK: /* 0 */ OPC_CheckField, 4, 4, 0,
// CHECK-NEXT: /* 4 */ OPC_Scope, 8, 0, // end scope at 15
// CHECK-NEXT: /* 7 */ OPC_CheckField, 2, 2, 0,
-// CHECK-NEXT: /* 11 */ OPC_TryDecode, {{[0-9]+}}, {{[0-9]+}}, 0,
+// CHECK-NEXT: /* 11 */ OPC_Decode, {{[0-9]+}}, {{[0-9]+}}, 0,
// CHECK-NEXT: /* 15 */ OPC_Decode, {{[0-9]+}}, {{[0-9]+}}, 1, // Opcode: InstA, DecodeIdx: 1
// CHECK-NEXT: };
@@ -47,7 +47,7 @@ def InstB : TestInstruction {
// CHECK-LARGE: /* 0 */ OPC_CheckField, 4, 4, 0,
// CHECK-LARGE-NEXT: /* 4 */ OPC_Scope, 8, 0, 0, // end scope at 16
// CHECK-LARGE-NEXT: /* 8 */ OPC_CheckField, 2, 2, 0,
-// CHECK-LARGE-NEXT: /* 12 */ OPC_TryDecode, {{[0-9]+}}, {{[0-9]+}}, 0,
+// CHECK-LARGE-NEXT: /* 12 */ OPC_Decode, {{[0-9]+}}, {{[0-9]+}}, 0,
// CHECK-LARGE-NEXT: /* 16 */ OPC_Decode, {{[0-9]+}}, {{[0-9]+}}, 1, // Opcode: InstA, DecodeIdx: 1
// CHECK-LARGE-NEXT: };
diff --git a/llvm/test/TableGen/DecoderEmitter/trydecode-emission4.td b/llvm/test/TableGen/DecoderEmitter/trydecode-emission4.td
index ff1a7e33747ba..2ff160cda2ec5 100644
--- a/llvm/test/TableGen/DecoderEmitter/trydecode-emission4.td
+++ b/llvm/test/TableGen/DecoderEmitter/trydecode-emission4.td
@@ -36,7 +36,7 @@ def InstB : TestInstruction {
// CHECK: /* 0 */ OPC_CheckField, 250, 3, 4, 0,
// CHECK-NEXT: /* 5 */ OPC_Scope, 9, 0, // end scope at 17
// CHECK-NEXT: /* 8 */ OPC_CheckField, 248, 3, 2, 0,
-// CHECK-NEXT: /* 13 */ OPC_TryDecode, {{[0-9]+}}, {{[0-9]+}}, 0,
+// CHECK-NEXT: /* 13 */ OPC_Decode, {{[0-9]+}}, {{[0-9]+}}, 0,
// CHECK-NEXT: /* 17 */ OPC_Decode, {{[0-9]+}}, {{[0-9]+}}, 1, // Opcode: InstA, DecodeIdx: 1
// CHECK-NEXT: };
@@ -46,7 +46,7 @@ def InstB : TestInstruction {
// CHECK-LARGE: /* 0 */ OPC_CheckField, 250, 3, 4, 0,
// CHECK-LARGE-NEXT: /* 5 */ OPC_Scope, 9, 0, 0, // end scope at 18
// CHECK-LARGE-NEXT: /* 9 */ OPC_CheckField, 248, 3, 2, 0,
-// CHECK-LARGE-NEXT: /* 14 */ OPC_TryDecode, {{[0-9]+}}, {{[0-9]+}}, 0,
+// CHECK-LARGE-NEXT: /* 14 */ OPC_Decode, {{[0-9]+}}, {{[0-9]+}}, 0,
// CHECK-LARGE-NEXT: /* 18 */ OPC_Decode, {{[0-9]+}}, {{[0-9]+}}, 1, // Opcode: InstA, DecodeIdx: 1
// CHECK-LARGE-NEXT: };
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 9aa7d31c62693..3585c244a72f0 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -228,7 +228,6 @@ struct DecoderTableInfo {
DecoderSet Decoders;
bool HasCheckPredicate;
bool HasSoftFail;
- bool HasTryDecode;
void insertPredicate(StringRef Predicate) {
Predicates.insert(CachedHashString(Predicate));
@@ -624,7 +623,6 @@ static StringRef getDecoderOpName(DecoderOps Op) {
CASE(OPC_CheckField);
CASE(OPC_CheckPredicate);
CASE(OPC_Decode);
- CASE(OPC_TryDecode);
CASE(OPC_SoftFail);
}
#undef CASE
@@ -792,8 +790,7 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS,
OS << "if !checkPredicate(" << PIdx << ") pop scope";
break;
}
- case OPC_Decode:
- case OPC_TryDecode: {
+ case OPC_Decode: {
// Decode the Opcode value.
unsigned Opc = DecodeAndEmitULEB128(I, OS);
@@ -1164,22 +1161,10 @@ void DecoderTableBuilder::emitSingletonTableEntry(
TableInfo.insertDecoder(Decoder);
unsigned DecoderIndex = TableInfo.getDecoderIndex(Decoder);
- // Produce OPC_Decode or OPC_TryDecode opcode based on the information
- // whether the instruction decoder is complete or not. If it is complete
- // then it handles all possible values of remaining variable/unfiltered bits
- // and for any value can determine if the bitpattern is a valid instruction
- // or not. This means OPC_Decode will be the final step in the decoding
- // process. If it is not complete, then the Fail return code from the
- // 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 DecoderOps DecoderOp =
- Encoding.hasCompleteDecoder() ? OPC_Decode : OPC_TryDecode;
- TableInfo.Table.insertOpcode(DecoderOp);
+ TableInfo.Table.insertOpcode(MCD::OPC_Decode);
const Record *InstDef = Encodings[EncodingID].getInstruction()->TheDef;
TableInfo.Table.insertULEB128(Target.getInstrIntValue(InstDef));
TableInfo.Table.insertULEB128(DecoderIndex);
- TableInfo.HasTryDecode |= DecoderOp == OPC_TryDecode;
}
std::unique_ptr<Filter>
@@ -1690,47 +1675,26 @@ static DecodeStatus decodeInstruction(const uint8_t DecodeTable[], MCInst &MI,
}
OS << R"(
S = decodeToMCInst(DecodeIdx, S, insn, MI, Address, DisAsm, DecodeComplete);
- assert(DecodeComplete);
-
LLVM_DEBUG(dbgs() << Loc << ": OPC_Decode: opcode " << Opc
<< ", using decoder " << DecodeIdx << ": "
- << (S != MCDisassembler::Fail ? "PASS\n" : "FAIL\n"));
- return S;
- })";
- if (TableInfo.HasTryDecode) {
- OS << R"(
- case OPC_TryDecode: {
- // Decode the Opcode value.
- unsigned Opc = decodeULEB128AndIncUnsafe(Ptr);
- unsigned DecodeIdx = decodeULEB128AndIncUnsafe(Ptr);
-
- // Perform the decode operation.
- MCInst TmpMI;
- TmpMI.setOpcode(Opc);
- bool DecodeComplete;
- S = decodeToMCInst(DecodeIdx, S, insn, TmpMI, Address, DisAsm, DecodeComplete);
- LLVM_DEBUG(dbgs() << Loc << ": OPC_TryDecode: opcode " << Opc
- << ", using decoder " << DecodeIdx << ": ");
+ << (S ? "PASS, " : "FAIL, "));
if (DecodeComplete) {
- // Decoding complete.
- LLVM_DEBUG(dbgs() << (S != MCDisassembler::Fail ? "PASS\n" : "FAIL\n"));
- MI = TmpMI;
+ LLVM_DEBUG(dbgs() << "decoding complete\n");
return S;
}
assert(S == MCDisassembler::Fail);
if (ScopeStack.empty()) {
- LLVM_DEBUG(dbgs() << "FAIL, returning FAIL\n");
+ LLVM_DEBUG(dbgs() << "returning Fail\n");
return MCDisassembler::Fail;
}
Ptr = ScopeStack.pop_back_val();
- LLVM_DEBUG(dbgs() << "FAIL, continuing at " << Ptr - DecodeTable << '\n');
+ LLVM_DEBUG(dbgs() << "continuing at " << Ptr - DecodeTable << '\n');
// Reset decode status. This also drops a SoftFail status that could be
// set before the decode attempt.
S = MCDisassembler::Success;
break;
})";
- }
if (TableInfo.HasSoftFail) {
OS << R"(
case OPC_SoftFail: {
|
I'd say that's not accurate. OPC Decode assumes that decoding will be always complete and is issued only for ops with a complete decoder. |
|
Change looks good though. |
jurahul
approved these changes
Sep 16, 2025
That's what I meant by "additional check" that is omitted in case of OPC_Decode (assumed to be always true). I'll try to rephrase. |
|
@jurahul I've updated the description, please check if it is more accurate now (or maybe you could suggest the proper wording?) |
LGTM, thanks. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/22719 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
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.
OPC_Decode is a specialized OPC_TryDecode. The difference between them
is that OPC_TryDecode performs a "completeness check", while OPC_Decode
asserts that the check passes.
The check is just a boolean test, which is nothing compared to the
complexity of the decoding process, so there is no point in having a
special opcode that optimizes the check.