[Mlir-commits] [mlir] 8975fb0 - [mlir] fix DiagnosedSilenceableFailure::takeDiagnostics

[Alex Zinenko]

Author: Alex Zinenko
Date: 2022-09-30T11:34:48Z
New Revision: 8975fb0b2647c5f2f0ba6460ca13c02ede109cd3

URL: https://github.com/llvm/llvm-project/commit/8975fb0b2647c5f2f0ba6460ca13c02ede109cd3
DIFF: https://github.com/llvm/llvm-project/commit/8975fb0b2647c5f2f0ba6460ca13c02ede109cd3.diff

LOG: [mlir] fix DiagnosedSilenceableFailure::takeDiagnostics

This function was returning an rvalue reference to an object that was
also cleared via RAII when the function returned, making it always
return an empty object. Make it accept the mutable reference to the
object instead to avoid this dangerous behavior.

Reviewed By: guraypp

Differential Revision: https://reviews.llvm.org/D134948

Added: 
    

Modified: 
    mlir/include/mlir/Dialect/Transform/IR/TransformInterfaces.h
    mlir/test/Dialect/Linalg/transform-op-pad.mlir
    mlir/test/Dialect/Transform/test-interpreter.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Dialect/Transform/IR/TransformInterfaces.h b/mlir/include/mlir/Dialect/Transform/IR/TransformInterfaces.h
index 3da82f01ba2a3..76fe4fadfc71e 100644
--- a/mlir/include/mlir/Dialect/Transform/IR/TransformInterfaces.h
+++ b/mlir/include/mlir/Dialect/Transform/IR/TransformInterfaces.h
@@ -125,14 +125,14 @@ class [[nodiscard]] DiagnosedSilenceableFailure {
     return result;
   }
 
-  /// Take the diagnostic and silence.
-  SmallVector<Diagnostic> &&takeDiagnostics() {
+  /// Take the diagnostics and silence.
+  void takeDiagnostics(SmallVectorImpl<Diagnostic> &diags) {
     assert(!diagnostics.empty() && "expected a diagnostic to be present");
-    auto guard = llvm::make_scope_exit([&]() { diagnostics.clear(); });
-    return std::move(diagnostics);
+    diags.append(std::make_move_iterator(diagnostics.begin()),
+                 std::make_move_iterator(diagnostics.end()));
   }
 
-  /// Streams the given values into the last diagnotic.
+  /// Streams the given values into the last diagnostic.
   /// Expects this object to be a silenceable failure.
   template <typename T>
   DiagnosedSilenceableFailure &operator<<(T &&value) & {
@@ -820,8 +820,7 @@ DiagnosedSilenceableFailure applyTransformToEach(
     if (result.isDefiniteFailure())
       return result;
     if (result.isSilenceableFailure())
-      for (auto &&diag : result.takeDiagnostics())
-        silenceableStack.push_back(std::move(diag));
+      result.takeDiagnostics(silenceableStack);
   }
   if (!silenceableStack.empty()) {
     return DiagnosedSilenceableFailure::silenceableFailure(

diff  --git a/mlir/test/Dialect/Linalg/transform-op-pad.mlir b/mlir/test/Dialect/Linalg/transform-op-pad.mlir
index d2d72cdd79e9f..58a3d6062924e 100644
--- a/mlir/test/Dialect/Linalg/transform-op-pad.mlir
+++ b/mlir/test/Dialect/Linalg/transform-op-pad.mlir
@@ -93,7 +93,7 @@ func.func @pad(%arg0: tensor<24x12xf32>,
 
 transform.with_pdl_patterns {
 ^bb0(%arg0: !pdl.operation):
-  transform.sequence %arg0 failures(propagate) {
+  transform.sequence %arg0 failures(suppress) {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.matmul"]} in %arg1
     // This error is silenceable and is not reported by this transform

diff  --git a/mlir/test/Dialect/Transform/test-interpreter.mlir b/mlir/test/Dialect/Transform/test-interpreter.mlir
index 0a5688f2239ce..e458864a18cb1 100644
--- a/mlir/test/Dialect/Transform/test-interpreter.mlir
+++ b/mlir/test/Dialect/Transform/test-interpreter.mlir
@@ -550,6 +550,7 @@ transform.with_pdl_patterns {
 
 func.func @foo() {
   "op" () { target_me } : () -> ()
+  // expected-note @below {{when applied to this op}}
   "op" () : () -> ()
   return
 }
@@ -566,6 +567,7 @@ transform.with_pdl_patterns {
   transform.sequence %arg0 failures(propagate) {
   ^bb0(%arg1: !pdl.operation):
     %0 = pdl_match @some in %arg1
+    // expected-error @below {{failed to apply}}
     transform.test_mixed_sucess_and_silenceable %0
   }
 }