[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

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

2282

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.

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;
 
+  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,

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 != (unsigned char) -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";
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!");
+        unsigned char TiedOp = OpInfo.TiedOperands.ResOpnd;
+        unsigned char SrcOp1 =
+            OpInfo.TiedOperands.SrcOpnd1Idx + HasMnemonicFirst;
+        unsigned char SrcOp2 =
+            OpInfo.TiedOperands.SrcOpnd2Idx + HasMnemonicFirst;
+        assert((i > TiedOp || TiedOp == (unsigned char)-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 unsigned char 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((unsigned char)std::get<0>(KV.first)) << ", "
+         << utostr((unsigned char)std::get<1>(KV.first)) << ", "
+         << utostr((unsigned char)std::get<2>(KV.first)) << " },\n";
     }
     OS << "};\n\n";
   } else
-    OS << "const char TiedAsmOperandTable[][3] = { /* empty  */ {0, 0, 0} "
-          "};\n\n";
+    OS << "const unsigned char 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";
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";

utils/TableGen/CodeGenInstruction.cpp

CodeGenInstAlias::CodeGenInstAlias(Record *R, unsigned Variant,
   // 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),