[mlir:PDL] Rework errors for pdl.operations with non-inferrable results

[rriddle]

Summary

We currently emit an error during verification if a pdl.operation with non-inferrable
results is used within a rewrite. This allows for catching some errors during compile
time, but is slightly broken. For one, the verification at the PDL level assumes that
all dialects have been loaded, which is true at run time, but may not be true when
the PDL is generated (such as via PDLL). This commit fixes this by not emitting the
error if the operation isn't registered, i.e. it uses the mightHave variant of trait/interface
methods.

Secondly, we currently don't verify when a pdl.operation has no explicit results, but the
operation being created is known to expect at least one. This commit adds a heuristic
error to detect these cases when possible and fail. We can't always capture when the user
made an error, but we can capture the most common case where the user expected an
operation to infer its result types (when it actually isn't possible).

Depends on D124582

[jpienaar]

mlir/lib/Dialect/PDL/IR/PDL.cpp

203

Assume here throws me a bit. This is the user wrote no results, we have nothing to contradict that, so accept they are telling the truth.

How about: don't attempt any verification for unknown op?

mlir/include/mlir/Dialect/PDL/IR/PDLOps.td

OpBuilder<(ins CArg<"Optional<StringRef>", "llvm::None">:$name,
             operandValues, attrValues, $_builder.getStrArrayAttr(attrNames),
             resultTypes);
     }]>,
   ];
   let extraClassDeclaration = [{
     /// Returns true if the operation type referenced supports result type
     /// inference.
     bool hasTypeInference();
+    
+    /// Returns true if the operation type referenced might support result type
+    /// inference, i.e. it supports type reference or is currently not
+    /// registered in the context. Returns false if the root operation name
+    /// has not been set.
+    bool mightHaveTypeInference();
   }];
   let hasVerifier = 1;
 }
 
 //===----------------------------------------------------------------------===//
 // pdl::PatternOp
 //===----------------------------------------------------------------------===//
 

mlir/include/mlir/IR/OperationSupport.h

public:
   /// Returns true if this operation has the given interface registered to it.
   template <typename T> bool hasInterface() const {
     return hasInterface(TypeID::get<T>());
   }
   bool hasInterface(TypeID interfaceID) const {
     return impl->interfaceMap.contains(interfaceID);
   }
 
+  /// Returns true if the operation *might* have the provided interface. This
+  /// means that either the operation is unregistered, or it was registered with
+  /// the provide interface.
+  template <typename T>
+  bool mightHaveInterface() const {
+    return mightHaveInterface(TypeID::get<T>());
+  }
+  bool mightHaveInterface(TypeID interfaceID) const {
+    return !isRegistered() || hasInterface(interfaceID);
+  }
+
   /// Return the dialect this operation is registered to if the dialect is
   /// loaded in the context, or nullptr if the dialect isn't loaded.
   Dialect *getDialect() const {
     return isRegistered() ? impl->dialect : impl->name.getReferencedDialect();
   }
 
   /// Return the name of the dialect this operation is registered to.
   StringRef getDialectNamespace() const;

mlir/lib/Dialect/PDL/IR/PDL.cpp

return replacedOp->getBlock() != rewriterBlock ||
            replacedOp->isBeforeInBlock(op);
   };
 
   // Check to see if the uses of the operation itself can be used to infer
   // types.
   if (llvm::any_of(op.op().getUses(), canInferTypeFromUse))
     return success();
 
+  // Handle the case where the operation has no explicit result types.
+  if (resultTypes.empty()) {
+    // If we don't know the concrete operation, don't attempt any verification.
+    // We can't make assumptions if we don't know the concrete operation.
+    Optional<StringRef> rawOpName = op.name();
+    if (!rawOpName)
+      return success();
+    Optional<RegisteredOperationName> opName =
+        RegisteredOperationName::lookup(*rawOpName, op.getContext());
+    if (!opName)
+      return success();
+
+    // If no explicit result types were provided, check to see if the operation
+    // expected at least one result. This doesn't cover all cases, but this
+    // should cover many cases in which the user intended to infer the results
+    // of an operation, but it isn't actually possible.
+    bool expectedAtLeastOneResult =
+        !opName->hasTrait<OpTrait::ZeroResult>() &&
+        !opName->hasTrait<OpTrait::VariadicResults>();
+    if (expectedAtLeastOneResult) {
+      return op
+          .emitOpError("must have inferable or constrained result types when "
+                       "nested within `pdl.rewrite`")
+          .attachNote()
+          .append("operation is created in a non-inferrable context, but '",
+                  *opName, "' does not implement InferTypeOpInterface");
+    }
+    return success();
+  }
+
   // Otherwise, make sure each of the types can be inferred.
   for (const auto &it : llvm::enumerate(resultTypes)) {
     Operation *resultTypeOp = it.value().getDefiningOp();
     assert(resultTypeOp && "expected valid result type operation");
 
     // If the op was defined by a `apply_native_rewrite`, it is guaranteed to be
     // usable.
     if (isa<ApplyNativeRewriteOp>(resultTypeOp))
return emitOpError()
            << "expected the same number of attribute values and attribute "
               "names, got "
            << attributeNames.size() << " names and " << attributeValues.size()
            << " values";
   }
 
   // If the operation is within a rewrite body and doesn't have type inference,
   // ensure that the result types can be resolved.
-  if (isWithinRewrite && !hasTypeInference()) {
+  if (isWithinRewrite && !mightHaveTypeInference()) {
     if (failed(verifyResultTypesAreInferrable(*this, types())))
       return failure();
   }
 
   return verifyHasBindingUse(*this);
 }
 
 bool OperationOp::hasTypeInference() {
-  Optional<StringRef> opName = name();
-  if (!opName)
-    return false;
+  if (Optional<StringRef> rawOpName = name()) {
+    OperationName opName(*rawOpName, getContext());
+    return opName.hasInterface<InferTypeOpInterface>();
+  }
+}
 
-  if (auto rInfo = RegisteredOperationName::lookup(*opName, getContext()))
-    return rInfo->hasInterface<InferTypeOpInterface>();
+bool OperationOp::mightHaveTypeInference() {
+  if (Optional<StringRef> rawOpName = name()) {
+    OperationName opName(*rawOpName, getContext());
+    return opName.mightHaveInterface<InferTypeOpInterface>();
+  }
   return false;
 }
 
 //===----------------------------------------------------------------------===//
 // pdl::PatternOp
 //===----------------------------------------------------------------------===//
 
 LogicalResult PatternOp::verifyRegions() {

mlir/test/Dialect/PDL/invalid.mlir

 
 pdl.pattern : benefit(1) {
   %op = operation "foo.op"
   rewrite %op {
     %type = type
 
     // expected-error@below {{op must have inferable or constrained result types when nested within `pdl.rewrite`}}
     // expected-note@below {{result type #0 was not constrained}}
-    %newOp = operation "foo.op" -> (%type : !pdl.type)
+    %newOp = operation "builtin.unrealized_conversion_cast" -> (%type : !pdl.type)
+  }
+}
+
+// -----
+
+// Unused operation only necessary to ensure the func dialect is loaded.
+func.func private @unusedOpToLoadFuncDialect()
+
+pdl.pattern : benefit(1) {
+  %op = operation "foo.op"
+  rewrite %op {
+    // expected-error@below {{op must have inferable or constrained result types when nested within `pdl.rewrite`}}
+    // expected-note@below {{operation is created in a non-inferrable context, but 'func.constant' does not implement InferTypeOpInterface}}
+    %newOp = operation "func.constant"
   }
 }
 
 // -----
 
 pdl.pattern : benefit(1) {
   // expected-error@below {{expected a bindable user when defined in the matcher body of a `pdl.pattern`}}
   %unused = operation "foo.op"