[mlir][transform] don't wrap a warning into silenceable failure

[ftynse]

Summary

Wrapping a warning into a silenceable failure will result in the warning
being interpreted as an error, which it is not.

mlir/lib/Dialect/Transform/IR/TransformOps.cpp

return implementSameInterface<transform::TransformHandleTypeInterface,
                                 transform::TransformValueHandleTypeInterface>(
       t1, t2);
 }
 
 /// Checks that the attributes of the function-like operation have correct
 /// consumption effect annotations. If `alsoVerifyInternal`, checks for
 /// annotations being present even if they can be inferred from the body.
 static DiagnosedSilenceableFailure
-verifyFunctionLikeConsumeAnnotations(FunctionOpInterface op,
+verifyFunctionLikeConsumeAnnotations(FunctionOpInterface op, bool emitWarnings,
                                      bool alsoVerifyInternal = false) {
   auto transformOp = cast<transform::TransformOpInterface>(op.getOperation());
   llvm::SmallDenseSet<unsigned> consumedArguments;
   if (!op.isExternal()) {
     transform::getConsumedBlockArguments(op.getFunctionBody().front(),
                                          consumedArguments);
   }
   for (unsigned i = 0, e = op.getNumArguments(); i < e; ++i) {
for (unsigned i = 0, e = op.getNumArguments(); i < e; ++i) {
     if (op.isExternal())
       continue;
 
     if (consumedArguments.contains(i) && !isConsumed && isReadOnly) {
       return transformOp.emitSilenceableError()
              << "argument #" << i
              << " is consumed in the body but is not marked as such";
     }
-    if (!consumedArguments.contains(i) && isConsumed) {
-      Diagnostic warning(op->getLoc(), DiagnosticSeverity::Warning);
-      warning << "argument #" << i
-              << " is not consumed in the body but is marked as consumed";
-      return DiagnosedSilenceableFailure::silenceableFailure(
-          std::move(warning));
+    if (emitWarnings && !consumedArguments.contains(i) && isConsumed) {
+      // Cannot use op.emitWarning() here as it would attempt to verify the op
+      // before printing, resulting in infinite recursion.
+      emitWarning(op->getLoc())
+          << "op argument #" << i
     }
   }
   return DiagnosedSilenceableFailure::success();
 }
 
 LogicalResult transform::ForeachMatchOp::verifySymbolUses(
     SymbolTableCollection &symbolTable) {
   assert(getMatchers().size() == getActions().size());
for (auto &&[matcher, action] :
     if (!matcherSymbol ||
         !isa<TransformOpInterface>(matcherSymbol.getOperation()))
       return emitError() << "unresolved matcher symbol " << matcher;
     if (!actionSymbol ||
         !isa<TransformOpInterface>(actionSymbol.getOperation()))
       return emitError() << "unresolved action symbol " << action;
 
     if (failed(verifyFunctionLikeConsumeAnnotations(matcherSymbol,
+                                                    /*emitWarnings=*/false,
                                                     /*alsoVerifyInternal=*/true)
                    .checkAndReport())) {
       return failure();
     }
     if (failed(verifyFunctionLikeConsumeAnnotations(actionSymbol,
+                                                    /*emitWarnings=*/false,
                                                     /*alsoVerifyInternal=*/true)
                    .checkAndReport())) {
       return failure();
     }
 
     ArrayRef<Type> matcherResults = matcherSymbol.getResultTypes();
     ArrayRef<Type> actionArguments = actionSymbol.getArgumentTypes();
     if (matcherResults.size() != actionArguments.size()) {
transform::IncludeOp::apply(transform::TransformRewriter &rewriter,
   detail::prepareValueMappings(
       mappings, callee.getBody().front().getTerminator()->getOperands(), state);
   for (auto &&[result, mapping] : llvm::zip_equal(getResults(), mappings))
     results.setMappedValues(result, mapping);
   return result;
 }
 
 static DiagnosedSilenceableFailure
-verifyNamedSequenceOp(transform::NamedSequenceOp op);
+verifyNamedSequenceOp(transform::NamedSequenceOp op, bool emitWarnings);
 
 void transform::IncludeOp::getEffects(
     SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
   // Always mark as modifying the payload.
   // TODO: a mechanism to annotate effects on payload. Even when all handles are
   // only read, the payload may still be modified, so we currently stay on the
   // conservative side and always indicate modification. This may prevent some
   // code reordering.
auto target =
       getOperation()->getAttrOfType<SymbolRefAttr>(getTargetAttrName());
   if (!target)
     return defaultEffects();
   auto callee = SymbolTable::lookupNearestSymbolFrom<NamedSequenceOp>(
       getOperation(), getTarget());
   if (!callee)
     return defaultEffects();
   DiagnosedSilenceableFailure earlyVerifierResult =
-      verifyNamedSequenceOp(callee);
+      verifyNamedSequenceOp(callee, /*emitWarnings=*/false);
   if (!earlyVerifierResult.succeeded()) {
     (void)earlyVerifierResult.silence();
     return defaultEffects();
   }
 
   for (unsigned i = 0, e = getNumOperands(); i < e; ++i) {
     if (callee.getArgAttr(i, TransformDialect::kArgConsumedAttrName))
       consumesHandle(getOperand(i), effects);
for (unsigned i = 0, e = fnType.getNumResults(); i != e; ++i) {
     if (!implementSameTransformInterface(resultType, funcType)) {
       return emitOpError() << "type of result #" << i
                            << " must implement the same transform dialect "
                               "interface as the corresponding callee result";
     }
   }
 
   return verifyFunctionLikeConsumeAnnotations(
-             cast<FunctionOpInterface>(*target),
+             cast<FunctionOpInterface>(*target), /*emitWarnings=*/false,
              /*alsoVerifyInternal=*/true)
       .checkAndReport();
 }
 
 //===----------------------------------------------------------------------===//
 // MatchOperationNameOp
 //===----------------------------------------------------------------------===//
 
verifyYieldingSingleBlockOp(FunctionOpInterface op, bool allowExternal) {
 
   return DiagnosedSilenceableFailure::success();
 }
 
 /// Verification of a NamedSequenceOp. This does not report the error
 /// immediately, so it can be used to check for op's well-formedness before the
 /// verifier runs, e.g., during trait verification.
 static DiagnosedSilenceableFailure
-verifyNamedSequenceOp(transform::NamedSequenceOp op) {
+verifyNamedSequenceOp(transform::NamedSequenceOp op, bool emitWarnings) {
   if (Operation *parent = op->getParentWithTrait<OpTrait::SymbolTable>()) {
     if (!parent->getAttr(
             transform::TransformDialect::kWithNamedSequenceAttrName)) {
       DiagnosedSilenceableFailure diag =
           emitSilenceableFailure(op)
           << "expects the parent symbol table to have the '"
           << transform::TransformDialect::kWithNamedSequenceAttrName
           << "' attribute";
       diag.attachNote(parent->getLoc()) << "symbol table operation";
       return diag;
     }
   }
 
   if (auto parent = op->getParentOfType<transform::TransformOpInterface>()) {
     DiagnosedSilenceableFailure diag =
         emitSilenceableFailure(op)
         << "cannot be defined inside another transform op";
     diag.attachNote(parent.getLoc()) << "ancestor transform op";
     return diag;
   }
 
   if (op.isExternal() || op.getBody().empty())
-    return verifyFunctionLikeConsumeAnnotations(cast<FunctionOpInterface>(*op));
+    return verifyFunctionLikeConsumeAnnotations(cast<FunctionOpInterface>(*op),
+                                                emitWarnings);
 
   if (op.getBody().front().empty())
     return emitSilenceableFailure(op) << "expected a non-empty body block";
 
   Operation *terminator = &op.getBody().front().back();
   if (!isa<transform::YieldOp>(terminator)) {
     DiagnosedSilenceableFailure diag = emitSilenceableFailure(op)
                                        << "expected '"
for (auto [i, operandType, resultType] :
     return emitSilenceableFailure(terminator)
            << "the type of the terminator operand #" << i
            << " must match the type of the corresponding parent op result ("
            << operandType << " vs " << resultType << ")";
   }
 
   auto funcOp = cast<FunctionOpInterface>(*op);
   DiagnosedSilenceableFailure diag =
-      verifyFunctionLikeConsumeAnnotations(funcOp);
+      verifyFunctionLikeConsumeAnnotations(funcOp, emitWarnings);
   if (!diag.succeeded())
     return diag;
 
   return verifyYieldingSingleBlockOp(funcOp,
                                      /*allowExternal=*/true);
 }
 
 LogicalResult transform::NamedSequenceOp::verify() {
   // Actual verification happens in a separate function for reusability.
-  return verifyNamedSequenceOp(*this).checkAndReport();
+  return verifyNamedSequenceOp(*this, /*emitWarnings=*/true).checkAndReport();
 }
 
 //===----------------------------------------------------------------------===//
 // SplitHandleOp
 //===----------------------------------------------------------------------===//
 
 void transform::SplitHandleOp::build(OpBuilder &builder, OperationState &result,
                                      Value target, int64_t numResultHandles) {

mlir/test/Dialect/Transform/ops-invalid.mlir

-// RUN: mlir-opt %s -split-input-file -verify-diagnostics
+// RUN: mlir-opt %s -split-input-file -verify-diagnostics | FileCheck %s
 
 // expected-error @below {{expects the entry block to have at least one argument}}
 transform.sequence failures(propagate) {
 }
 
 // -----
 
 // expected-error @below {{expects the first entry block argument to be of type implementing TransformHandleTypeInterface}}
^bb0(%arg0: !transform.any_op):
     transform.include @foo failures(propagate) (%arg0) : (!transform.any_op) -> ()
     transform.yield
   }
 }
 
 // -----
 
 module attributes { transform.with_named_sequence } {
+  // Note that printing a warning doesn't result in verification failures, so this
+  // also checks for the IR being printed back.
+  // CHECK-LABEL: transform.named_sequence @emit_warning_only
   // expected-warning @below {{argument #0 is not consumed in the body but is marked as consume}}
-  transform.named_sequence @foo(%op: !transform.any_op {transform.consumed}) {
+  transform.named_sequence @emit_warning_only(%op: !transform.any_op {transform.consumed}) {
     transform.test_print_remark_at_operand %op, "message" : !transform.any_op
     transform.yield
   }
 
   transform.sequence failures(propagate) {
   ^bb0(%arg0: !transform.any_op):
-    transform.include @foo failures(propagate) (%arg0) : (!transform.any_op) -> ()
+    transform.include @emit_warning_only failures(propagate) (%arg0) : (!transform.any_op) -> ()
     transform.yield
   }
 }
 
 // -----
 
 module attributes { transform.with_named_sequence } {
   // expected-error @below {{argument #0 is consumed in the body but is not marked as such}}