[TableGen][AsmMatcherEmitter] Allow tied operands of different classes in aliases.

[sdesmalen]

Summary

Allow a tied operand of a different operand class in InstAliases,
so that the operand can be printed (and added to the MC instruction)
as the appropriate register. For example, 'GPR64as32', which would
be printed/parsed as a 32bit register and should match a tied 64bit
register operand, where the former is a sub-register of the latter.

This patch also generalizes the constraint checking to an overrideable
method in MCTargetAsmParser, so that target asmparsers can specify
whether a given operand satisfies the tied register constraint.

[sdesmalen]

Friendly ping..

[fhahn]

To provide a bit more info here, this patch helps to use tied operands to handle instruction like SVE's sqinc (D47716)

sqinc x0, w0, all, mul #4
       ^___^ (must match)

Although I am not entirely sure if that is the best way, I could not think of a better approach right now and it should do the trick. It would be great to hear some additional thoughts. In any case, I think it would be great to document this behavior somewhere, otherwise it will be hard for other people to find and use.

[fhahn]

LGTM, given that it solves the problem as a small extension of tied operand handling. Please wait with committing till next week, in case anyone else has comments or suggestions.

I suppose there is no place where to document this behavior, but could you add a brief comment to regsEqual, stating what it is used for?

utils/TableGen/AsmMatcherEmitter.cpp

2162 ↗

(On Diff #149748)

If we are unsigned char here, should we change the type of TiedOperandsEnumMap's key accordingly?

2282 ↗

(On Diff #149748)

Do we need the casts here? The elements in TiedOperandsEnumMap are unsigned already?

[sdesmalen]

Thanks @fhahn I've added a comment to regsEqual describing what it can be used for.

llvm/trunk/include/llvm/MC/MCParser/MCTargetAsmParser.h

 //===- llvm/MC/MCTargetAsmParser.h - Target Assembly Parser -----*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_MC_MCPARSER_MCTARGETASMPARSER_H
 #define LLVM_MC_MCPARSER_MCTARGETASMPARSER_H
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCParser/MCAsmLexer.h"
+#include "llvm/MC/MCParser/MCParsedAsmOperand.h"
 #include "llvm/MC/MCParser/MCAsmParserExtension.h"
 #include "llvm/MC/MCTargetOptions.h"
 #include "llvm/Support/SMLoc.h"
 #include <cstdint>
 #include <memory>
 
 namespace llvm {
 
public:
   /// any complex target predicates not expressible via match classes.
   virtual unsigned checkTargetMatchPredicate(MCInst &Inst) {
     return Match_Success;
   }
 
   virtual void convertToMapAndConstraints(unsigned Kind,
                                           const OperandVector &Operands) = 0;
 
+  /// Returns whether two registers are equal and is used by the tied-operands
+  /// checks in the AsmMatcher. This method can be overridden allow e.g. a
+  /// sub- or super-register as the tied operand.
+  virtual bool regsEqual(const MCParsedAsmOperand &Op1,
+                         const MCParsedAsmOperand &Op2) const {
+    assert(Op1.isReg() && Op2.isReg() && "Operands not all regs");
+    return Op1.getReg() == Op2.getReg();
+  }
+
   // Return whether this parser uses assignment statements with equals tokens
   virtual bool equalIsAsmAssignment() { return true; };
   // Return whether this start of statement identifier is a label
   virtual bool isLabel(AsmToken &Token) { return true; };
   // Return whether this parser accept star as start of statement
   virtual bool starIsStartOfStatement() { return false; };
 
   virtual const MCExpr *applyModifierToExpr(const MCExpr *E,

llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp

if (TiedOp != -1) {
         if (AliasConstraintsAreChecked)
           Insert.first->second = SrcOp2;
 
         // In case it only has one reference in the asm string,
         // it doesn't need to be checked for tied constraints.
         SrcOp2 = (SrcOp2 == (unsigned)-1) ? SrcOp1 : SrcOp2;
       }
 
+      // If the alias operand is of a different operand class, we only want
+      // to benefit from the tied-operands check and just match the operand
+      // as a normal, but not copy the original (TiedOp) to the result
+      // instruction. We do this by passing -1 as the tied operand to copy.
+      if (ResultInst->Operands[i].Rec->getName() !=
+          ResultInst->Operands[TiedOp].Rec->getName()) {
+        SrcOp1 = ResOperands[TiedOp].AsmOperandNum;
+        int SubIdx = CGA.ResultInstOperandIndex[AliasOpNo].second;
+        StringRef Name = CGA.ResultOperands[AliasOpNo].getName();
+        SrcOp2 = findAsmOperand(Name, SubIdx);
+        ResOperands.push_back(
+            ResOperand::getTiedOp((unsigned)-1, SrcOp1, SrcOp2));
+      } else {
         ResOperands.push_back(ResOperand::getTiedOp(TiedOp, SrcOp1, SrcOp2));
         continue;
       }
+    }
 
     // Handle all the suboperands for this operand.
     const std::string &OpName = OpInfo->Name;
     for ( ; AliasOpNo <  LastOpNo &&
             CGA.ResultInstOperandIndex[AliasOpNo].first == i; ++AliasOpNo) {
       int SubIdx = CGA.ResultInstOperandIndex[AliasOpNo].second;
 
       // Find out what operand from the asmparser that this MCInst operand
static void emitConvertFuncs(CodeGenTarget &Target, StringRef ClassName,
   CvtOS << "      static_cast<" << TargetOperandClass
         << "&>(*Operands[OpIdx]).addRegOperands(Inst, 1);\n";
   CvtOS << "      break;\n";
   CvtOS << "    case CVT_Tied: {\n";
   CvtOS << "      assert(OpIdx < (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 << "      Inst.addOperand(Inst.getOperand(TiedResOpnd));\n";
+  CvtOS << "      if (TiedResOpnd != (uint8_t) -1)\n";
+  CvtOS << "        Inst.addOperand(Inst.getOperand(TiedResOpnd));\n";
   CvtOS << "      break;\n";
   CvtOS << "    }\n";
 
   std::string OperandFnBody;
   raw_string_ostream OpOS(OperandFnBody);
   // Start the operand number lookup function.
   OpOS << "void " << Target.getName() << ClassName << "::\n"
        << "convertToMapAndConstraints(unsigned Kind,\n";
static void emitConvertFuncs(CodeGenTarget &Target, StringRef ClassName,
   // Pre-populate the operand conversion kinds with the standard always
   // available entries.
   OperandConversionKinds.insert(CachedHashString("CVT_Done"));
   OperandConversionKinds.insert(CachedHashString("CVT_Reg"));
   OperandConversionKinds.insert(CachedHashString("CVT_Tied"));
   enum { CVT_Done, CVT_Reg, CVT_Tied };
 
   // Map of e.g. <0, 2, 3> -> "Tie_0_2_3" enum label.
-  std::map<std::tuple<unsigned, unsigned, unsigned>, std::string>
+  std::map<std::tuple<uint8_t, uint8_t, uint8_t>, std::string>
   TiedOperandsEnumMap;
 
   for (auto &II : Infos) {
     // Check if we have a custom match function.
     StringRef AsmMatchConverter =
         II->getResultInst()->TheDef->getValueAsString("AsmMatchConverter");
     if (!AsmMatchConverter.empty() && II->UseInstAsmMatchConverter) {
       std::string Signature = ("ConvertCustom_" + AsmMatchConverter).str();
for (unsigned i = 0, e = II->ResOperands.size(); i != e; ++i) {
         OpOS << "      NumMCOperands += " << OpInfo.MINumOperands << ";\n"
              << "      break;\n";
         break;
       }
       case MatchableInfo::ResOperand::TiedOperand: {
         // If this operand is tied to a previous one, just copy the MCInst
         // operand from the earlier one.We can only tie single MCOperand values.
         assert(OpInfo.MINumOperands == 1 && "Not a singular MCOperand");
-        unsigned TiedOp = OpInfo.TiedOperands.ResOpnd;
-        unsigned SrcOp1 = OpInfo.TiedOperands.SrcOpnd1Idx + HasMnemonicFirst;
-        unsigned SrcOp2 = OpInfo.TiedOperands.SrcOpnd2Idx + HasMnemonicFirst;
-        assert(i > TiedOp && "Tied operand precedes its target!");
+        uint8_t TiedOp = OpInfo.TiedOperands.ResOpnd;
+        uint8_t SrcOp1 =
+            OpInfo.TiedOperands.SrcOpnd1Idx + HasMnemonicFirst;
+        uint8_t SrcOp2 =
+            OpInfo.TiedOperands.SrcOpnd2Idx + HasMnemonicFirst;
+        assert((i > TiedOp || TiedOp == (uint8_t)-1) &&
+               "Tied operand precedes its target!");
         auto TiedTupleName = std::string("Tie") + utostr(TiedOp) + '_' +
                              utostr(SrcOp1) + '_' + utostr(SrcOp2);
         Signature += "__" + TiedTupleName;
         ConversionRow.push_back(CVT_Tied);
         ConversionRow.push_back(TiedOp);
         ConversionRow.push_back(SrcOp1);
         ConversionRow.push_back(SrcOp2);
 
static void emitConvertFuncs(CodeGenTarget &Target, StringRef ClassName,
 
   // Finish up the operand number lookup function.
   OpOS << "    }\n  }\n}\n\n";
 
   // Output a static table for tied operands.
   if (TiedOperandsEnumMap.size()) {
     // The number of tied operand combinations will be small in practice,
     // but just add the assert to be sure.
-    assert(TiedOperandsEnumMap.size() <= 255 &&
+    assert(TiedOperandsEnumMap.size() <= 254 &&
            "Too many tied-operand combinations to reference with "
-           "an 8bit offset from the conversion table");
+           "an 8bit offset from the conversion table, where index "
+           "'255' is reserved as operand not to be copied.");
 
     OS << "enum {\n";
     for (auto &KV : TiedOperandsEnumMap) {
       OS << "  " << KV.second << ",\n";
     }
     OS << "};\n\n";
 
-    OS << "const char TiedAsmOperandTable[][3] = {\n";
+    OS << "const uint8_t TiedAsmOperandTable[][3] = {\n";
     for (auto &KV : TiedOperandsEnumMap) {
-      OS << "  /* " << KV.second << " */ { " << std::get<0>(KV.first) << ", "
-         << std::get<1>(KV.first) << ", " << std::get<2>(KV.first) << " },\n";
+      OS << "  /* " << KV.second << " */ { "
+         << utostr(std::get<0>(KV.first)) << ", "
+         << utostr(std::get<1>(KV.first)) << ", "
+         << utostr(std::get<2>(KV.first)) << " },\n";
     }
     OS << "};\n\n";
   } else
-    OS << "const char TiedAsmOperandTable[][3] = { /* empty  */ {0, 0, 0} "
-          "};\n\n";
+    OS << "const uint8_t TiedAsmOperandTable[][3] = "
+          "{ /* empty  */ {0, 0, 0} };\n\n";
 
   OS << "namespace {\n";
 
   // Output the operand conversion kind enum.
   OS << "enum OperatorConversionKind {\n";
   for (const auto &Converter : OperandConversionKinds)
     OS << "  " << Converter << ",\n";
   OS << "  CVT_NUM_CONVERTERS\n";
for (unsigned i = 0, e = ConversionTable[Row].size(); i != e; i += 2) {
           CachedHashString("CVT_Tied")) {
         OS << (unsigned)(ConversionTable[Row][i + 1]) << ", ";
         continue;
       }
 
       // For a tied operand, emit a reference to the TiedAsmOperandTable
       // that contains the operand to copy, and the parsed operands to
       // check for their tied constraints.
-      auto Key = std::make_tuple((unsigned)ConversionTable[Row][i + 1],
-                                 (unsigned)ConversionTable[Row][i + 2],
-                                 (unsigned)ConversionTable[Row][i + 3]);
+      auto Key = std::make_tuple((uint8_t)ConversionTable[Row][i + 1],
+                                 (uint8_t)ConversionTable[Row][i + 2],
+                                 (uint8_t)ConversionTable[Row][i + 3]);
       auto TiedOpndEnum = TiedOperandsEnumMap.find(Key);
       assert(TiedOpndEnum != TiedOperandsEnumMap.end() &&
              "No record for tied operand pair");
       OS << TiedOpndEnum->second << ", ";
       i += 2;
     }
     OS << "CVT_Done },\n";
   }
static void emitCustomOperandParsing(raw_ostream &OS, CodeGenTarget &Target,
   OS << "  // Okay, we had no match.\n";
   OS << "  return MatchOperand_NoMatch;\n";
   OS << "}\n\n";
 }
 
 static void emitAsmTiedOperandConstraints(CodeGenTarget &Target,
                                           AsmMatcherInfo &Info,
                                           raw_ostream &OS) {
+  std::string AsmParserName =
+      Info.AsmParser->getValueAsString("AsmParserClassName");
   OS << "static bool ";
-  OS << "checkAsmTiedOperandConstraints(unsigned Kind,\n";
+  OS << "checkAsmTiedOperandConstraints(const " << Target.getName()
+     << AsmParserName << "&AsmParser,\n";
+  OS << "                               unsigned Kind,\n";
   OS << "                               const OperandVector &Operands,\n";
   OS << "                               uint64_t &ErrorInfo) {\n";
   OS << "  assert(Kind < CVT_NUM_SIGNATURES && \"Invalid signature!\");\n";
   OS << "  const uint8_t *Converter = ConversionTable[Kind];\n";
   OS << "  for (const uint8_t *p = Converter; *p; p+= 2) {\n";
   OS << "    switch (*p) {\n";
   OS << "    case CVT_Tied: {\n";
   OS << "      unsigned OpIdx = *(p+1);\n";
   OS << "      assert(OpIdx < (size_t)(std::end(TiedAsmOperandTable) -\n";
   OS << "                              std::begin(TiedAsmOperandTable)) &&\n";
   OS << "             \"Tied operand not found\");\n";
   OS << "      unsigned OpndNum1 = TiedAsmOperandTable[OpIdx][1];\n";
   OS << "      unsigned OpndNum2 = TiedAsmOperandTable[OpIdx][2];\n";
   OS << "      if (OpndNum1 != OpndNum2) {\n";
   OS << "        auto &SrcOp1 = Operands[OpndNum1];\n";
   OS << "        auto &SrcOp2 = Operands[OpndNum2];\n";
-  OS << "        if (SrcOp1->isReg() && SrcOp2->isReg() &&\n";
-  OS << "            SrcOp1->getReg() != SrcOp2->getReg()) {\n";
-  OS << "          ErrorInfo = OpndNum2;\n";
-  OS << "          return false;\n";
-  OS << "        }\n";
-  OS << "      }\n";
+  OS << "        if (SrcOp1->isReg() && SrcOp2->isReg()) {\n";
+  OS << "          if (!AsmParser.regsEqual(*SrcOp1, *SrcOp2)) {\n";
+  OS << "            ErrorInfo = OpndNum2;\n";
+  OS << "            return false;\n";
+  OS << "          }\n";
+  OS << "        }\n";
+  OS << "      }\n";
   OS << "      break;\n";
   OS << "    }\n";
   OS << "    default:\n";
   OS << "      break;\n";
   OS << "    }\n";
   OS << "  }\n";
   OS << "  return true;\n";
   OS << "}\n\n";
if (ReportMultipleNearMisses) {
     OS << "      }\n";
     OS << "      continue;\n";
     OS << "    }\n\n";
   }
 
   OS << "    if (matchingInlineAsm) {\n";
   OS << "      convertToMapAndConstraints(it->ConvertFn, Operands);\n";
   if (!ReportMultipleNearMisses) {
-    OS << "      if (!checkAsmTiedOperandConstraints(it->ConvertFn, Operands, ErrorInfo))\n";
+    OS << "      if (!checkAsmTiedOperandConstraints(*this, it->ConvertFn, "
+          "Operands, ErrorInfo))\n";
     OS << "        return Match_InvalidTiedOperand;\n";
     OS << "\n";
   }
   OS << "      return Match_Success;\n";
   OS << "    }\n\n";
   OS << "    // We have selected a definite instruction, convert the parsed\n"
      << "    // operands into the appropriate MCInst.\n";
   if (HasOptionalOperands) {
if (HasDeprecation) {
     OS << "        MII.get(Inst.getOpcode()).getDeprecatedInfo(Inst, getSTI(), Info)) {\n";
     OS << "      SMLoc Loc = ((" << Target.getName()
        << "Operand&)*Operands[0]).getStartLoc();\n";
     OS << "      getParser().Warning(Loc, Info, None);\n";
     OS << "    }\n";
   }
 
   if (!ReportMultipleNearMisses) {
-    OS << "    if (!checkAsmTiedOperandConstraints(it->ConvertFn, Operands, ErrorInfo))\n";
+    OS << "    if (!checkAsmTiedOperandConstraints(*this, it->ConvertFn, "
+          "Operands, ErrorInfo))\n";
     OS << "      return Match_InvalidTiedOperand;\n";
     OS << "\n";
   }
 
   OS << "    DEBUG_WITH_TYPE(\n";
   OS << "        \"asm-matcher\",\n";
   OS << "        dbgs() << \"Opcode result: complete match, selecting this opcode\\n\");\n";
   OS << "    return Match_Success;\n";

llvm/trunk/utils/TableGen/CodeGenInstruction.cpp

CodeGenInstAlias::CodeGenInstAlias(Record *R, CodeGenTarget &T)
   // Decode and validate the arguments of the result.
   unsigned AliasOpNo = 0;
   for (unsigned i = 0, e = ResultInst->Operands.size(); i != e; ++i) {
 
     // Tied registers don't have an entry in the result dag unless they're part
     // of a complex operand, in which case we include them anyways, as we
     // don't have any other way to specify the whole operand.
     if (ResultInst->Operands[i].MINumOperands == 1 &&
-        ResultInst->Operands[i].getTiedRegister() != -1)
+        ResultInst->Operands[i].getTiedRegister() != -1) {
+      // Tied operands of different RegisterClass should be explicit within an
+      // instruction's syntax and so cannot be skipped.
+      int TiedOpNum = ResultInst->Operands[i].getTiedRegister();
+      if (ResultInst->Operands[i].Rec->getName() ==
+          ResultInst->Operands[TiedOpNum].Rec->getName())
         continue;
+    }
 
     if (AliasOpNo >= Result->getNumArgs())
       PrintFatalError(R->getLoc(), "not enough arguments for instruction!");
 
     Record *InstOpRec = ResultInst->Operands[i].Rec;
     unsigned NumSubOps = ResultInst->Operands[i].MINumOperands;
     ResultOperand ResOp(static_cast<int64_t>(0));
     if (tryAliasOpMatch(Result, AliasOpNo, InstOpRec, (NumSubOps > 1),