[PATCH] D35998: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true

[Alexandru Guduleasa via Phabricator via llvm-commits]

alexandru.guduleasa created this revision.

Consider the following instruction: "inst.eq $dst, $src" where ".eq" is an optional flag operand.
The $src and $dst operands are registers.
If we parse the instruction "inst r0, r1", the flag is not present and it will be marked in the "OptionalOperandsMask" variable.
After the matching is complete we call the "convertToMCInst" method.

The current implementation works only if the optional operands are at the end of the array.
The "Operands" array looks like [token:"inst", reg:r0, reg:r1].
The first operand that must be added to the MCInst is the destination, the r0 register.
The "OpIdx" (in the Operands array) for this register is 2.
However, since the flag is not present in the Operands, the actual index for r0 should be 1.
The flag is not present since we rely on the default value.

This patch removes the "NumDefaults" variable and replaces it with an array (DefaultsOffset).
This array contains an index for each operand (excluding the mnemonic).
At each index, the array contains the number of optional operands that should be subtracted.
For the previous example, this array looks like this: [0, 1, 1].
When we need to access the r0 register, we compute its index as 2 - DefaultsOffset[1] = 1.


https://reviews.llvm.org/D35998

Files:
  utils/TableGen/AsmMatcherEmitter.cpp


Index: utils/TableGen/AsmMatcherEmitter.cpp
===================================================================
--- utils/TableGen/AsmMatcherEmitter.cpp
+++ utils/TableGen/AsmMatcherEmitter.cpp
@@ -1847,13 +1847,24 @@
   CvtOS << "  assert(Kind < CVT_NUM_SIGNATURES && \"Invalid signature!\");\n";
   CvtOS << "  const uint8_t *Converter = ConversionTable[Kind];\n";
   if (HasOptionalOperands) {
-    CvtOS << "  unsigned NumDefaults = 0;\n";
+    size_t MaxNumOperands = 0;
+    for (const auto &MI : Infos) {
+      MaxNumOperands = std::max(MaxNumOperands, MI->AsmOperands.size());
+    }
+    CvtOS << "  unsigned DefaultsOffset[" << (MaxNumOperands) << "];\n";
+    CvtOS << "  assert(OptionalOperandsMask.size() == " << (MaxNumOperands)
+          << ");\n";
+    CvtOS << "  for (unsigned i = 0, NumDefaults = 0; i < " << (MaxNumOperands)
+          << "; ++i) {\n";
+    CvtOS << "    DefaultsOffset[i] = NumDefaults;\n";
+    CvtOS << "    NumDefaults += (OptionalOperandsMask[i] ? 1 : 0);\n";
+    CvtOS << "  }\n";
   }
   CvtOS << "  unsigned OpIdx;\n";
   CvtOS << "  Inst.setOpcode(Opcode);\n";
   CvtOS << "  for (const uint8_t *p = Converter; *p; p+= 2) {\n";
   if (HasOptionalOperands) {
-    CvtOS << "    OpIdx = *(p + 1) - NumDefaults;\n";
+    CvtOS << "    OpIdx = *(p + 1) - DefaultsOffset[*(p + 1) - 1];\n";
   } else {
     CvtOS << "    OpIdx = *(p + 1);\n";
   }
@@ -1988,7 +1999,6 @@
                 << "        " << Op.Class->DefaultMethod << "()"
                 << "->" << Op.Class->RenderMethod << "(Inst, "
                 << OpInfo.MINumOperands << ");\n"
-                << "        ++NumDefaults;\n"
                 << "      } else {\n"
                 << "        static_cast<" << TargetOperandClass
                 << "&>(*Operands[OpIdx])." << Op.Class->RenderMethod


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35998.108648.patch
Type: text/x-patch
Size: 1816 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170728/bed5928b/attachment.bin>