[globalisel][tablegen] Report more detail in some SelectionDAG import failures. NFC

[ab]

A couple minor nits, but LGTM.

utils/TableGen/GlobalISelEmitter.cpp

1334–1348

Can you also outline this one into some sort of "explainOperator"?

1336–1345

All of these cases seem obvious from the name; maybe print them all as '(<operator>)' ?

1628

Have isTrivialOperatorNode return an Error instead? That lets you do something like:

if (auto Err = checkTrivialOperatorNode(Dst))

return failedImport("Dst pattern root isn't a trivial operator (" + toString(Err) + ")");

1642

Is llvm:: needed?

[dsanders]

Update to account for nits

utils/TableGen/GlobalISelEmitter.cpp

static Optional<LLTCodeGen> MVTToLLT(MVT::SimpleValueType SVT) {
   MVT VT(SVT);
   if (VT.isVector() && VT.getVectorNumElements() != 1)
     return LLTCodeGen(LLT::vector(VT.getVectorNumElements(), VT.getScalarSizeInBits()));
   if (VT.isInteger() || VT.isFloatingPoint())
     return LLTCodeGen(LLT::scalar(VT.getSizeInBits()));
   return None;
 }
 
-static bool isTrivialOperatorNode(const TreePatternNode *N) {
-  return !N->isLeaf() && !N->hasAnyPredicate() && !N->getTransformFn();
+static std::string explainPredicates(const TreePatternNode *N) {
+  std::string Explanation = "";
+  StringRef Separator = "";
+  for (const auto &P : N->getPredicateFns()) {
+    Explanation +=
+        (Separator + P.getOrigPatFragRecord()->getRecord()->getName()).str();
+    if (P.isAlwaysTrue())
+      Explanation += " always-true";
+    if (P.isImmediatePattern())
+      Explanation += " immediate";
+  }
+  return Explanation;
+}
+
+static std::string explainRulePredicates(const ArrayRef<Init *> Predicates) {
+  std::string Explanation = "";
+  StringRef Separator = "";
+  for (const auto *P : Predicates) {
+    Explanation += Separator;
+
+    if (const DefInit *PDef = dyn_cast<DefInit>(P)) {
+      Explanation += PDef->getDef()->getName();
+    } else
+      Explanation += "<unknown>";
+  }
+  return Explanation;
+}
+
+std::string explainOperator(Record *Operator) {
+  if (Operator->isSubClassOf("SDNode"))
+    return " (" + Operator->getValueAsString("Opcode") + ")";
+
+  if (Operator->isSubClassOf("Intrinsic"))
+    return (" (Operator is an Intrinsic, " + Operator->getName() + ")").str();
+
+  return " (Operator not understood)";
+}
+
+/// Helper function to let the emitter report skip reason error messages.
+static Error failedImport(const Twine &Reason) {
+  return make_error<StringError>(Reason, inconvertibleErrorCode());
+}
+
+static Error isTrivialOperatorNode(const TreePatternNode *N) {
+  std::string Explanation = "";
+  std::string Separator = "";
+  if (N->isLeaf()) {
+    Explanation = "Is a leaf";
+    Separator = ", ";
+  }
+
+  if (N->hasAnyPredicate()) {
+    Explanation = Separator + "Has a predicate (" + explainPredicates(N) + ")";
+    Separator = ", ";
+  }
+
+  if (N->getTransformFn()) {
+    Explanation += Separator + "Has a transform function";
+    Separator = ", ";
+  }
+
+  if (!N->isLeaf() && !N->hasAnyPredicate() && !N->getTransformFn())
+    return Error::success();
+
+  return failedImport(Explanation);
 }
 
 //===- Matchers -----------------------------------------------------------===//
 
 class OperandMatcher;
 class MatchAction;
 
 /// Generates code to check that a match rule matches.
const CodeGenInstruction *GlobalISelEmitter::findNodeEquiv(Record *N) const {
   return NodeEquivs.lookup(N);
 }
 
 GlobalISelEmitter::GlobalISelEmitter(RecordKeeper &RK)
     : RK(RK), CGP(RK), Target(CGP.getTargetInfo()) {}
 
 //===- Emitter ------------------------------------------------------------===//
 
-/// Helper function to let the emitter report skip reason error messages.
-static Error failedImport(const Twine &Reason) {
-  return make_error<StringError>(Reason, inconvertibleErrorCode());
-}
-
 Error
 GlobalISelEmitter::importRulePredicates(RuleMatcher &M,
                                         ArrayRef<Init *> Predicates) const {
   if (!Predicates.empty())
-    return failedImport("Pattern has a predicate");
+    return failedImport("Pattern has a rule predicate (" +
+                        explainRulePredicates(Predicates) + ")");
   return Error::success();
 }
 
 Expected<InstructionMatcher &> GlobalISelEmitter::createAndImportSelDAGMatcher(
     InstructionMatcher &InsnMatcher, const TreePatternNode *Src) const {
   // Start with the defined operands (i.e., the results of the root operator).
   if (Src->getExtTypes().size() > 1)
     return failedImport("Src pattern has multiple results");
 
   auto SrcGIOrNull = findNodeEquiv(Src->getOperator());
   if (!SrcGIOrNull)
-    return failedImport("Pattern operator lacks an equivalent Instruction");
+    return failedImport("Pattern operator lacks an equivalent Instruction" +
+                        explainOperator(Src->getOperator()));
   auto &SrcGI = *SrcGIOrNull;
 
   // The operators look good: match the opcode and mutate it to the new one.
   InsnMatcher.addPredicate<InstructionOpcodeMatcher>(&SrcGI);
 
   unsigned OpIdx = 0;
   unsigned TempOpIdx = 0;
   for (const EEVT::TypeSet &Ty : Src->getExtTypes()) {
     auto OpTyOrNone = MVTToLLT(Ty.getConcrete());
 
     if (!OpTyOrNone)
       return failedImport(
           "Result of Src pattern operator has an unsupported type");
 
     // Results don't have a name unless they are the root node. The caller will
     // set the name if appropriate.
     OperandMatcher &OM = InsnMatcher.addOperand(OpIdx++, "", TempOpIdx);
     OM.addPredicate<LLTOperandMatcher>(*OpTyOrNone);
   }
 
   // Match the used operands (i.e. the children of the operator).
   for (unsigned i = 0, e = Src->getNumChildren(); i != e; ++i) {
     if (auto Error = importChildMatcher(InsnMatcher, Src->getChild(i), OpIdx++,
                                         TempOpIdx))
       return std::move(Error);
   }
 
   return InsnMatcher;
 }
 
 Error GlobalISelEmitter::importChildMatcher(InstructionMatcher &InsnMatcher,
                                             TreePatternNode *SrcChild,
                                             unsigned OpIdx,
                                             unsigned &TempOpIdx) const {
   OperandMatcher &OM =
       InsnMatcher.addOperand(OpIdx, SrcChild->getName(), TempOpIdx);
 
   if (SrcChild->hasAnyPredicate())
-    return failedImport("Src pattern child has predicate");
+    return failedImport("Src pattern child has predicate (" +
+                        explainPredicates(SrcChild) + ")");
 
   ArrayRef<EEVT::TypeSet> ChildTypes = SrcChild->getExtTypes();
   if (ChildTypes.size() != 1)
     return failedImport("Src pattern child has multiple results");
 
   // Check MBB's before the type check since they are not a known type.
   if (!SrcChild->isLeaf()) {
     if (SrcChild->getOperator()->isSubClassOf("SDNode")) {
if (ChildRec->isSubClassOf("RegisterClass")) {
           Target.getRegisterClass(ChildRec));
       return Error::success();
     }
 
     // Check for ComplexPattern's.
     if (ChildRec->isSubClassOf("ComplexPattern")) {
       const auto &ComplexPattern = ComplexPatternEquivs.find(ChildRec);
       if (ComplexPattern == ComplexPatternEquivs.end())
-        return failedImport(
-            "SelectionDAG ComplexPattern not mapped to GlobalISel");
+        return failedImport("SelectionDAG ComplexPattern (" +
+                            ChildRec->getName() + ") not mapped to GlobalISel");
 
       const auto &Predicate = OM.addPredicate<ComplexPatternOperandMatcher>(
           OM, *ComplexPattern->second);
       TempOpIdx += Predicate.countTemporaryOperands();
       return Error::success();
     }
 
+    if (ChildRec->isSubClassOf("ImmLeaf")) {
+      return failedImport(
+          "Src pattern child def is an unsupported tablegen class (ImmLeaf)");
+    }
+
     return failedImport(
         "Src pattern child def is an unsupported tablegen class");
   }
 
   return failedImport("Src pattern child is an unsupported kind");
 }
 
 Error GlobalISelEmitter::importExplicitUseRenderer(
if (DstChild->getOperator()->isSubClassOf("SDNode")) {
         return Error::success();
       }
     }
     return failedImport("Dst pattern child isn't a leaf node or an MBB");
   }
 
   // Otherwise, we're looking for a bog-standard RegisterClass operand.
   if (DstChild->hasAnyPredicate())
-    return failedImport("Dst pattern child has predicate");
+    return failedImport("Dst pattern child has predicate (" +
+                        explainPredicates(DstChild) + ")");
 
   if (auto *ChildDefInit = dyn_cast<DefInit>(DstChild->getLeafValue())) {
     auto *ChildRec = ChildDefInit->getDef();
 
     ArrayRef<EEVT::TypeSet> ChildTypes = DstChild->getExtTypes();
     if (ChildTypes.size() != 1)
       return failedImport("Dst pattern child has multiple results");
 
if (ChildRec->isSubClassOf("ComplexPattern")) {
       for (unsigned I = 0; I < OM.countTemporaryOperands(); ++I)
         RenderedOperands.push_back(OperandPlaceholder::CreateTemporary(
             OM.getAllocatedTemporariesBaseID() + I));
       DstMIBuilder.addRenderer<RenderComplexPatternOperand>(
           *ComplexPattern->second, RenderedOperands);
       return Error::success();
     }
 
+    if (ChildRec->isSubClassOf("SDNodeXForm"))
+      return failedImport("Dst pattern child def is an unsupported tablegen "
+                          "class (SDNodeXForm)");
+
     return failedImport(
         "Dst pattern child def is an unsupported tablegen class");
   }
 
   return failedImport("Dst pattern child is an unsupported kind");
 }
 
 Expected<BuildMIAction &> GlobalISelEmitter::createAndImportInstructionRenderer(
     RuleMatcher &M, const TreePatternNode *Dst,
     const InstructionMatcher &InsnMatcher) const {
   Record *DstOp = Dst->getOperator();
-  if (!DstOp->isSubClassOf("Instruction"))
+  if (!DstOp->isSubClassOf("Instruction")) {
+    if (DstOp->isSubClassOf("ValueType"))
+      return failedImport(
+          "Pattern operator isn't an instruction (it's a ValueType)");
     return failedImport("Pattern operator isn't an instruction");
+  }
   auto &DstI = Target.getInstruction(DstOp);
 
   auto &DstMIBuilder = M.addAction<BuildMIAction>(&DstI, InsnMatcher);
 
   // Render the explicit defs.
   for (unsigned I = 0; I < DstI.Operands.NumDefs; ++I) {
     const auto &DstIOperand = DstI.Operands[I];
     DstMIBuilder.addRenderer<CopyRenderer>(InsnMatcher, DstIOperand.Name);
Expected<RuleMatcher> GlobalISelEmitter::runOnPattern(const PatternToMatch &P) {
   if (auto Error = importRulePredicates(M, P.getPredicates()->getValues()))
     return std::move(Error);
 
   // Next, analyze the pattern operators.
   TreePatternNode *Src = P.getSrcPattern();
   TreePatternNode *Dst = P.getDstPattern();
 
   // If the root of either pattern isn't a simple operator, ignore it.
-  if (!isTrivialOperatorNode(Dst))
-    return failedImport("Dst pattern root isn't a trivial operator");
-  if (!isTrivialOperatorNode(Src))
-    return failedImport("Src pattern root isn't a trivial operator");
+  if (auto Err = isTrivialOperatorNode(Dst))
+    return failedImport("Dst pattern root isn't a trivial operator (" +
+                        toString(std::move(Err)) + ")");
+  if (auto Err = isTrivialOperatorNode(Src))
+    return failedImport("Src pattern root isn't a trivial operator (" +
+                        toString(std::move(Err)) + ")");
 
   // Start with the defined operands (i.e., the results of the root operator).
   Record *DstOp = Dst->getOperator();
   if (!DstOp->isSubClassOf("Instruction"))
     return failedImport("Pattern operator isn't an instruction");
 
   auto &DstI = Target.getInstruction(DstOp);
   if (DstI.Operands.NumDefs != Src->getExtTypes().size())
-    return failedImport("Src pattern results and dst MI defs are different");
+    return failedImport("Src pattern results and dst MI defs are different (" +
+                        to_string(Src->getExtTypes().size()) + " def(s) vs " +
+                        to_string(DstI.Operands.NumDefs) + " def(s))");
 
   InstructionMatcher &InsnMatcherTemp = M.addInstructionMatcher();
   auto InsnMatcherOrError = createAndImportSelDAGMatcher(InsnMatcherTemp, Src);
   if (auto Error = InsnMatcherOrError.takeError())
     return std::move(Error);
   InstructionMatcher &InsnMatcher = InsnMatcherOrError.get();
 
   // The root of the match also has constraints on the register bank so that it