Skip to content

Conversation

s-barannikov
Copy link
Contributor

@s-barannikov s-barannikov commented Aug 30, 2025

Use information from CGIOperandList instead of re-parsing operand dags from scratch.

@llvmbot
Copy link
Member

llvmbot commented Aug 30, 2025

@llvm/pr-subscribers-tablegen

Author: Sergei Barannikov (s-barannikov)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/156181.diff

1 Files Affected:

  • (modified) llvm/utils/TableGen/DecoderEmitter.cpp (+15-47)
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index f1db0368d0988..98569abfbb1a4 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -1937,19 +1937,6 @@ static void addOneOperandFields(const Record *EncodingDef, const BitsInit &Bits,
 }
 
 void InstructionEncoding::parseFixedLenOperands(const BitsInit &Bits) {
-  const Record &Def = *Inst->TheDef;
-
-  // Gather the outputs/inputs of the instruction, so we can find their
-  // positions in the encoding.  This assumes for now that they appear in the
-  // MCInst in the order that they're listed.
-  std::vector<std::pair<const Init *, StringRef>> InOutOperands;
-  const DagInit *Out = Def.getValueAsDag("OutOperandList");
-  const DagInit *In = Def.getValueAsDag("InOperandList");
-  for (const auto &[Idx, Arg] : enumerate(Out->getArgs()))
-    InOutOperands.emplace_back(Arg, Out->getArgNameStr(Idx));
-  for (const auto &[Idx, Arg] : enumerate(In->getArgs()))
-    InOutOperands.emplace_back(Arg, In->getArgNameStr(Idx));
-
   // Search for tied operands, so that we can correctly instantiate
   // operands that are not explicitly represented in the encoding.
   std::map<StringRef, StringRef> TiedNames;
@@ -1972,48 +1959,28 @@ void InstructionEncoding::parseFixedLenOperands(const BitsInit &Bits) {
   }
 
   // For each operand, see if we can figure out where it is encoded.
-  for (const auto &Op : InOutOperands) {
-    const Init *OpInit = Op.first;
-    StringRef OpName = Op.second;
-
-    // We're ready to find the instruction encoding locations for this
-    // operand.
-
-    // First, find the operand type ("OpInit"), and sub-op names
-    // ("SubArgDag") if present.
-    const DagInit *SubArgDag = dyn_cast<DagInit>(OpInit);
-    if (SubArgDag)
-      OpInit = SubArgDag->getOperator();
-    const Record *OpTypeRec = cast<DefInit>(OpInit)->getDef();
-    // Lookup the sub-operands from the operand type record (note that only
-    // Operand subclasses have MIOperandInfo, see CodeGenInstruction.cpp).
-    const DagInit *SubOps = OpTypeRec->isSubClassOf("Operand")
-                                ? OpTypeRec->getValueAsDag("MIOperandInfo")
-                                : nullptr;
-
+  for (const CGIOperandList::OperandInfo &Op : Inst->Operands) {
     // Lookup the decoder method and construct a new OperandInfo to hold our
     // result.
-    OperandInfo OpInfo = getOpInfo(OpTypeRec);
+    OperandInfo OpInfo = getOpInfo(Op.Rec);
 
     // If we have named sub-operands...
-    if (SubArgDag) {
+    if (Op.MIOperandInfo && !Op.SubOpNames[0].empty()) {
       // Then there should not be a custom decoder specified on the top-level
       // type.
       if (!OpInfo.Decoder.empty()) {
         PrintError(EncodingDef,
-                   "DecoderEmitter: operand \"" + OpName + "\" has type \"" +
-                       OpInit->getAsString() +
+                   "DecoderEmitter: operand \"" + Op.Name + "\" has type \"" +
+                       Op.Rec->getName() +
                        "\" with a custom DecoderMethod, but also named "
                        "sub-operands.");
         continue;
       }
 
       // Decode each of the sub-ops separately.
-      assert(SubOps && SubArgDag->getNumArgs() == SubOps->getNumArgs());
-      for (const auto &[I, Arg] : enumerate(SubOps->getArgs())) {
-        StringRef SubOpName = SubArgDag->getArgNameStr(I);
-        OperandInfo SubOpInfo = getOpInfo(cast<DefInit>(Arg)->getDef());
-
+      for (auto [SubOpName, SubOp] :
+           zip_equal(Op.SubOpNames, Op.MIOperandInfo->getArgs())) {
+        OperandInfo SubOpInfo = getOpInfo(cast<DefInit>(SubOp)->getDef());
         addOneOperandFields(EncodingDef, Bits, TiedNames, SubOpName, SubOpInfo);
         Operands.push_back(std::move(SubOpInfo));
       }
@@ -2022,17 +1989,18 @@ void InstructionEncoding::parseFixedLenOperands(const BitsInit &Bits) {
 
     // Otherwise, if we have an operand with sub-operands, but they aren't
     // named...
-    if (SubOps && OpInfo.Decoder.empty()) {
+    if (Op.MIOperandInfo && OpInfo.Decoder.empty()) {
       // If it's a single sub-operand, and no custom decoder, use the decoder
       // from the one sub-operand.
-      if (SubOps->getNumArgs() == 1)
-        OpInfo = getOpInfo(cast<DefInit>(SubOps->getArg(0))->getDef());
+      if (Op.MIOperandInfo->getNumArgs() == 1)
+        OpInfo =
+            getOpInfo(cast<DefInit>(Op.MIOperandInfo->getArg(0))->getDef());
 
       // If we have multiple sub-ops, there'd better have a custom
       // decoder. (Otherwise we don't know how to populate them properly...)
-      if (SubOps->getNumArgs() > 1) {
+      if (Op.MIOperandInfo->getNumArgs() > 1) {
         PrintError(EncodingDef,
-                   "DecoderEmitter: operand \"" + OpName +
+                   "DecoderEmitter: operand \"" + Op.Name +
                        "\" uses MIOperandInfo with multiple ops, but doesn't "
                        "have a custom decoder!");
         debugDumpRecord(*EncodingDef);
@@ -2040,7 +2008,7 @@ void InstructionEncoding::parseFixedLenOperands(const BitsInit &Bits) {
       }
     }
 
-    addOneOperandFields(EncodingDef, Bits, TiedNames, OpName, OpInfo);
+    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

@s-barannikov s-barannikov enabled auto-merge (squash) August 30, 2025 13:53
@s-barannikov s-barannikov merged commit cacab8a into llvm:main Aug 30, 2025
11 checks passed
@s-barannikov s-barannikov deleted the tablegen/decoder/simplify-parse-fixed-operands branch August 30, 2025 14:31
@llvm-ci
Copy link
Collaborator

llvm-ci commented Aug 30, 2025

LLVM Buildbot has detected a new failure on builder clang-aarch64-quick running on linaro-clang-aarch64-quick while building llvm at step 5 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/21990

Here is the relevant piece of the build log for the reference
Step 5 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'Clangd Unit Tests :: ./ClangdTests/72/332' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests-Clangd Unit Tests-3697611-72-332.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=332 GTEST_SHARD_INDEX=72 /home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests
--

Script:
--
/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests --gtest_filter=TUSchedulerTests.Debounce
--
ASTWorker building file /clangd-test/foo.cpp version null with command 
[/clangd-test]
clang -ffreestanding /clangd-test/foo.cpp
Driver produced command: cc1 -cc1 -triple aarch64-unknown-linux-gnu -fsyntax-only -disable-free -clear-ast-before-backend -main-file-name foo.cpp -mrelocation-model pic -pic-level 2 -pic-is-pie -mframe-pointer=non-leaf -fmath-errno -ffp-contract=on -fno-rounding-math -mconstructor-aliases -ffreestanding -enable-tlsdesc -target-cpu generic -target-feature +v8a -target-feature +fp-armv8 -target-feature +neon -target-abi aapcs -debugger-tuning=gdb -fdebug-compilation-dir=/clangd-test -fcoverage-compilation-dir=/clangd-test -resource-dir lib/clang/22 -internal-isystem lib/clang/22/include -internal-isystem /usr/local/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -fdeprecated-macro -ferror-limit 19 -fno-signed-char -fgnuc-version=4.2.1 -fskip-odr-check-in-gmf -fcxx-exceptions -fexceptions -no-round-trip-args -target-feature -fmv -faddrsig -D__GCC_HAVE_DWARF2_CFI_ASM=1 -x c++ /clangd-test/foo.cpp
Building first preamble for /clangd-test/foo.cpp version null
Built preamble of size 736848 for file /clangd-test/foo.cpp version null in 2.55 seconds
../llvm/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:266: Failure
Failed
auto should have been debounced and canceled

ASTWorker building file /clangd-test/foo.cpp version null with command 
[/clangd-test]
clang -ffreestanding /clangd-test/foo.cpp
Driver produced command: cc1 -cc1 -triple aarch64-unknown-linux-gnu -fsyntax-only -disable-free -clear-ast-before-backend -main-file-name foo.cpp -mrelocation-model pic -pic-level 2 -pic-is-pie -mframe-pointer=non-leaf -fmath-errno -ffp-contract=on -fno-rounding-math -mconstructor-aliases -ffreestanding -enable-tlsdesc -target-cpu generic -target-feature +v8a -target-feature +fp-armv8 -target-feature +neon -target-abi aapcs -debugger-tuning=gdb -fdebug-compilation-dir=/clangd-test -fcoverage-compilation-dir=/clangd-test -resource-dir lib/clang/22 -internal-isystem lib/clang/22/include -internal-isystem /usr/local/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -fdeprecated-macro -ferror-limit 19 -fno-signed-char -fgnuc-version=4.2.1 -fskip-odr-check-in-gmf -fcxx-exceptions -fexceptions -no-round-trip-args -target-feature -fmv -faddrsig -D__GCC_HAVE_DWARF2_CFI_ASM=1 -x c++ /clangd-test/foo.cpp
Reusing preamble version null for version null of /clangd-test/foo.cpp
ASTWorker building file /clangd-test/foo.cpp version null with command 
[/clangd-test]
clang -ffreestanding /clangd-test/foo.cpp
Driver produced command: cc1 -cc1 -triple aarch64-unknown-linux-gnu -fsyntax-only -disable-free -clear-ast-before-backend -main-file-name foo.cpp -mrelocation-model pic -pic-level 2 -pic-is-pie -mframe-pointer=non-leaf -fmath-errno -ffp-contract=on -fno-rounding-math -mconstructor-aliases -ffreestanding -enable-tlsdesc -target-cpu generic -target-feature +v8a -target-feature +fp-armv8 -target-feature +neon -target-abi aapcs -debugger-tuning=gdb -fdebug-compilation-dir=/clangd-test -fcoverage-compilation-dir=/clangd-test -resource-dir lib/clang/22 -internal-isystem lib/clang/22/include -internal-isystem /usr/local/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -fdeprecated-macro -ferror-limit 19 -fno-signed-char -fgnuc-version=4.2.1 -fskip-odr-check-in-gmf -fcxx-exceptions -fexceptions -no-round-trip-args -target-feature -fmv -faddrsig -D__GCC_HAVE_DWARF2_CFI_ASM=1 -x c++ /clangd-test/foo.cpp

../llvm/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:266
Failed
auto should have been debounced and canceled



********************


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants