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

[Ahmed Bougacha via Phabricator via llvm-commits]

ab accepted this revision.
ab added a comment.
This revision is now accepted and ready to land.

A couple minor nits, but LGTM.



================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:1307-1321
+    std::string Explanation = "";
+
+    if (Src->getOperator()->isSubClassOf("SDNode")) {
+      std::string Opcode = Src->getOperator()->getValueAsString("Opcode");
+      if (StringRef(Opcode).startswith("ISD::"))
+        Explanation = " (Operator is a standard SDNode, " + Opcode + ")";
+      else
----------------
Can you also outline this one into some sort of "explainOperator"?


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:1309-1318
+    if (Src->getOperator()->isSubClassOf("SDNode")) {
+      std::string Opcode = Src->getOperator()->getValueAsString("Opcode");
+      if (StringRef(Opcode).startswith("ISD::"))
+        Explanation = " (Operator is a standard SDNode, " + Opcode + ")";
+      else
+        Explanation = " (Operator is a custom SDNode, " + Opcode + ")";
+    } else if (Src->getOperator()->isSubClassOf("Intrinsic"))
----------------
All of these cases seem obvious from the name;  maybe print them all as '(<operator>)' ?


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:1616
+  std::string ExtraReason;
+  if (!isTrivialOperatorNode(Dst, ExtraReason))
+    return failedImport("Dst pattern root isn't a trivial operator (" +
----------------
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) + ")");


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:1631
+    return failedImport("Src pattern results and dst MI defs are different (" +
+                        llvm::to_string(Src->getExtTypes().size()) + " def(s) vs " +
+                        llvm::to_string(DstI.Operands.NumDefs) + " def(s))");
----------------
Is llvm:: needed?


https://reviews.llvm.org/D31325