[llvm] [TableGen] Bug fix for tied optional operands resolution (PR #83588)

[Alfie Richards via llvm-commits]

https://github.com/AlfieRichardsArm updated https://github.com/llvm/llvm-project/pull/83588

>From 60edb61db647e881f96e57c450671b5bf617cd3a Mon Sep 17 00:00:00 2001
From: Alfie Richards <alfie.richards at arm.com>
Date: Fri, 1 Mar 2024 15:30:40 +0000
Subject: [PATCH 1/3] [TableGen] Bug fix for tied optional operands resolution

---
 llvm/utils/TableGen/AsmMatcherEmitter.cpp | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/llvm/utils/TableGen/AsmMatcherEmitter.cpp b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
index febd96086df27b..e0602a71707a73 100644
--- a/llvm/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
@@ -1986,9 +1986,9 @@ emitConvertFuncs(CodeGenTarget &Target, StringRef ClassName,
   }
   CvtOS << "  assert(Kind < CVT_NUM_SIGNATURES && \"Invalid signature!\");\n";
   CvtOS << "  const uint8_t *Converter = ConversionTable[Kind];\n";
-  CvtOS << "  unsigned OpIdx;\n";
   CvtOS << "  Inst.setOpcode(Opcode);\n";
   CvtOS << "  for (const uint8_t *p = Converter; *p; p += 2) {\n";
+  CvtOS << "    unsigned OpIdx;\n";
   if (HasOptionalOperands) {
     // When optional operands are involved, formal and actual operand indices
     // may differ. Map the former to the latter by subtracting the number of
@@ -1999,16 +1999,17 @@ emitConvertFuncs(CodeGenTarget &Target, StringRef ClassName,
   }
   CvtOS << "    switch (*p) {\n";
   CvtOS << "    default: llvm_unreachable(\"invalid conversion entry!\");\n";
-  CvtOS << "    case CVT_Reg:\n";
+  CvtOS << "    case CVT_Reg:{\n";
   CvtOS << "      static_cast<" << TargetOperandClass
         << " &>(*Operands[OpIdx]).addRegOperands(Inst, 1);\n";
   CvtOS << "      break;\n";
+  CvtOS << "    }\n";
   CvtOS << "    case CVT_Tied: {\n";
-  CvtOS << "      assert(OpIdx < (size_t)(std::end(TiedAsmOperandTable) -\n";
+  CvtOS << "      assert(*(p + 1) < (size_t)(std::end(TiedAsmOperandTable) -\n";
   CvtOS
       << "                              std::begin(TiedAsmOperandTable)) &&\n";
   CvtOS << "             \"Tied operand not found\");\n";
-  CvtOS << "      unsigned TiedResOpnd = TiedAsmOperandTable[OpIdx][0];\n";
+  CvtOS << "      unsigned TiedResOpnd = TiedAsmOperandTable[*(p + 1)][0];\n";
   CvtOS << "      if (TiedResOpnd != (uint8_t)-1)\n";
   CvtOS << "        Inst.addOperand(Inst.getOperand(TiedResOpnd));\n";
   CvtOS << "      break;\n";

>From ac16abbcb467f2f60969b35c6acb43a6efa3e6ff Mon Sep 17 00:00:00 2001
From: Alfie Richards <alfie.richards at arm.com>
Date: Thu, 14 Mar 2024 09:14:36 +0000
Subject: [PATCH 2/3] Address comments

---
 llvm/utils/TableGen/AsmMatcherEmitter.cpp | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/llvm/utils/TableGen/AsmMatcherEmitter.cpp b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
index e0602a71707a73..217cd94711b090 100644
--- a/llvm/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
@@ -1988,22 +1988,20 @@ emitConvertFuncs(CodeGenTarget &Target, StringRef ClassName,
   CvtOS << "  const uint8_t *Converter = ConversionTable[Kind];\n";
   CvtOS << "  Inst.setOpcode(Opcode);\n";
   CvtOS << "  for (const uint8_t *p = Converter; *p; p += 2) {\n";
-  CvtOS << "    unsigned OpIdx;\n";
   if (HasOptionalOperands) {
     // When optional operands are involved, formal and actual operand indices
     // may differ. Map the former to the latter by subtracting the number of
     // absent optional operands.
-    CvtOS << "    OpIdx = *(p + 1) - DefaultsOffset[*(p + 1)];\n";
+    CvtOS << "    unsigned OpIdx = *(p + 1) - DefaultsOffset[*(p + 1)];\n";
   } else {
-    CvtOS << "    OpIdx = *(p + 1);\n";
+    CvtOS << "    unsigned OpIdx = *(p + 1);\n";
   }
   CvtOS << "    switch (*p) {\n";
   CvtOS << "    default: llvm_unreachable(\"invalid conversion entry!\");\n";
-  CvtOS << "    case CVT_Reg:{\n";
+  CvtOS << "    case CVT_Reg:\n";
   CvtOS << "      static_cast<" << TargetOperandClass
         << " &>(*Operands[OpIdx]).addRegOperands(Inst, 1);\n";
   CvtOS << "      break;\n";
-  CvtOS << "    }\n";
   CvtOS << "    case CVT_Tied: {\n";
   CvtOS << "      assert(*(p + 1) < (size_t)(std::end(TiedAsmOperandTable) -\n";
   CvtOS

>From c162aa28465e3b6ae86977b62c0f6e25b11cda18 Mon Sep 17 00:00:00 2001
From: Alfie Richards <alfie.richards at arm.com>
Date: Thu, 14 Mar 2024 10:19:40 +0000
Subject: [PATCH 3/3] Add FIXME comment

---
 llvm/utils/TableGen/AsmMatcherEmitter.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/llvm/utils/TableGen/AsmMatcherEmitter.cpp b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
index 217cd94711b090..3ded07921b8241 100644
--- a/llvm/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
@@ -1992,6 +1992,7 @@ emitConvertFuncs(CodeGenTarget &Target, StringRef ClassName,
     // When optional operands are involved, formal and actual operand indices
     // may differ. Map the former to the latter by subtracting the number of
     // absent optional operands.
+    // FIXME: This is not an operand index in the CVT_Tied case
     CvtOS << "    unsigned OpIdx = *(p + 1) - DefaultsOffset[*(p + 1)];\n";
   } else {
     CvtOS << "    unsigned OpIdx = *(p + 1);\n";