[mlir-tblgen] Slightly improve the diagnostic message in pattern match

[jpienaar]

mlir/tools/mlir-tblgen/RewriterGen.cpp

262

Can you use /*matchStr=*/ ?

345

The message and comment above it is difficult to reconcile. Does that comment apply somewhere else?

[Chia-hungDuan]

Addressed review comment

[Chia-hungDuan]

mlir/tools/mlir-tblgen/RewriterGen.cpp

345

After checked the history, the logic mentioned by the comment has been moved above in the llvm::dyn_cast_or_null(in this commit ac68637ba94d05f413b1b963950c300fb2f81a99), so this one can be removed.

[Chia-hungDuan]

Fix a bug that calling emitMatchCheck to do null check for the op who is
supposed to report that match failure, e.g.,

// This is invalid.
emitMatchCheck(op, /*matchStr=*/op, ...);

mlir/tools/mlir-tblgen/RewriterGen.cpp

void PatternEmitter::emitNativeCodeMatch(DagNode tree, StringRef opName,
   LLVM_DEBUG(llvm::dbgs() << '\n');
 
   // TODO(suderman): iterate through arguments, determine their types, output
   // names.
   SmallVector<std::string, 8> capture;
 
   raw_indented_ostream::DelimitedScope scope(os);
 
-  os << "if(!" << opName << ") return ::mlir::failure();\n";
   for (int i = 0, e = tree.getNumArgs(); i != e; ++i) {
     std::string argName = formatv("arg{0}_{1}", depth, i);
     if (DagNode argTree = tree.getArgAsNestedDag(i)) {
       os << "Value " << argName << ";\n";
     } else {
       auto leaf = tree.getArgAsLeaf(i);
       if (leaf.isAttrMatcher() || leaf.isConstantAttr()) {
         os << "Attribute " << argName << ";\n";
       } else {
         os << "Value " << argName << ";\n";
       }
     }
 
     capture.push_back(std::move(argName));
   }
 
   bool hasLocationDirective;
   std::string locToUse;
   std::tie(hasLocationDirective, locToUse) = getLocation(tree);
 
   auto fmt = tree.getNativeCodeTemplate();
-  if (fmt.count("$_self") != 1) {
+  if (fmt.count("$_self") != 1)
     PrintFatalError(loc, "NativeCodeCall must have $_self as argument for "
                          "passing the defining Operation");
-  }
 
   auto nativeCodeCall = std::string(tgfmt(
       fmt, &fmtCtx.addSubst("_loc", locToUse).withSelf(opName.str()), capture));
 
-  os << "if (failed(" << nativeCodeCall << ")) return ::mlir::failure();\n";
+  emitMatchCheck(opName, formatv("!failed({0})", nativeCodeCall),
+                 formatv("\"{0} return failure\"", nativeCodeCall));
 
   for (int i = 0, e = tree.getNumArgs(); i != e; ++i) {
     auto name = tree.getArgName(i);
     if (!name.empty() && name != "_") {
       os << formatv("{0} = {1};\n", name, capture[i]);
     }
   }
 

 // Helper function to match patterns.
 void PatternEmitter::emitOpMatch(DagNode tree, StringRef opName, int depth) {
   Operator &op = tree.getDialectOp(opMap);
   LLVM_DEBUG(llvm::dbgs() << "start emitting match for op '"
                           << op.getOperationName() << "' at depth " << depth
                           << '\n');
 
   std::string castedName = formatv("castedOp{0}", depth);
-  os << formatv("auto {0} = ::llvm::dyn_cast_or_null<{2}>({1}); "
+  os << formatv("auto {0} = ::llvm::dyn_cast<{2}>({1}); "
                 "(void){0};\n",
                 castedName, opName, op.getQualCppClassName());
+
   // Skip the operand matching at depth 0 as the pattern rewriter already does.
-  if (depth != 0) {
-    // Skip if there is no defining operation (e.g., arguments to function).
-    os << formatv("if (!{0}) return ::mlir::failure();\n", castedName);
-  }
-  if (tree.getNumArgs() != op.getNumArgs()) {
+  if (depth != 0)
+    emitMatchCheck(opName, /*matchStr=*/castedName,
+                   formatv("\"{0} is not {1} type\"", castedName,
+                           op.getQualCppClassName()));
+
+  if (tree.getNumArgs() != op.getNumArgs())
     PrintFatalError(loc, formatv("op '{0}' argument number mismatch: {1} in "
                                  "pattern vs. {2} in definition",
                                  op.getOperationName(), tree.getNumArgs(),
                                  op.getNumArgs()));
-  }
 
   // If the operand's name is set, set to that variable.
   auto name = tree.getSymbol();
   if (!name.empty())
     os << formatv("{0} = {1};\n", name, castedName);
 
   for (int i = 0, e = tree.getNumArgs(), nextOperand = 0; i != e; ++i) {
     auto opArg = op.getArg(i);
if (DagNode argTree = tree.getArgAsNestedDag(i)) {
       }
       os << "{\n";
 
       // Attributes don't count for getODSOperands.
       // TODO: Operand is a Value, check if we should remove `getDefiningOp()`.
       os.indent() << formatv(
           "auto *{0} = "
           "(*{1}.getODSOperands({2}).begin()).getDefiningOp();\n",
-          argName, castedName, nextOperand++);
+          argName, castedName, nextOperand);
+      // Null check of operand's definingOp
+      emitMatchCheck(castedName, /*matchStr=*/argName,
+                     formatv("\"Operand {0} of {1} has null definingOp\"",
+                             nextOperand++, castedName));
       emitMatch(argTree, argName, depth + 1);
       os << formatv("tblgen_ops[{0}] = {1};\n", ++opCounter, argName);
       os.unindent() << "}\n";
       continue;
     }
 
     // Next handle DAG leaf: operand or attribute
     if (opArg.is<NamedTypeConstraint *>()) {