[Mlir-commits] [mlir] 83892d7 - Print custom assembly on pass failure by default

[Mehdi Amini]

Author: Mehdi Amini
Date: 2022-04-19T17:29:08Z
New Revision: 83892d76f460402b45729e10b2219563b01d71b4

URL: https://github.com/llvm/llvm-project/commit/83892d76f460402b45729e10b2219563b01d71b4
DIFF: https://github.com/llvm/llvm-project/commit/83892d76f460402b45729e10b2219563b01d71b4.diff

LOG: Print custom assembly on pass failure by default

The printer is now resilient to invalid IR and will already automatically
fallback to the generic form on invalid IR. Using the generic printer on
pass failure was a conservative option before the printer was made
failsafe.

Reviewed By: lattner, rriddle, jpienaar, bondhugula

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

Added: 
    mlir/test/Pass/invalid-ir-print-after-failure.mlir

Modified: 
    mlir/lib/Pass/IRPrinting.cpp
    mlir/test/Pass/ir-printing.mlir
    mlir/test/lib/Pass/TestPassManager.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Pass/IRPrinting.cpp b/mlir/lib/Pass/IRPrinting.cpp
index 9e1e56ba4cb63..dff65b6fe77c5 100644
--- a/mlir/lib/Pass/IRPrinting.cpp
+++ b/mlir/lib/Pass/IRPrinting.cpp
@@ -170,8 +170,7 @@ void IRPrinterInstrumentation::runAfterPassFailed(Pass *pass, Operation *op) {
 
   config->printAfterIfEnabled(pass, op, [&](raw_ostream &out) {
     out << formatv("// -----// IR Dump After {0} Failed", pass->getName());
-    printIR(op, config->shouldPrintAtModuleScope(), out,
-            OpPrintingFlags().printGenericOpForm());
+    printIR(op, config->shouldPrintAtModuleScope(), out, OpPrintingFlags());
     out << "\n\n";
   });
 }

diff  --git a/mlir/test/Pass/invalid-ir-print-after-failure.mlir b/mlir/test/Pass/invalid-ir-print-after-failure.mlir
new file mode 100644
index 0000000000000..f205550cd3207
--- /dev/null
+++ b/mlir/test/Pass/invalid-ir-print-after-failure.mlir
@@ -0,0 +1,12 @@
+// RUN: not mlir-opt %s -pass-pipeline='func.func(test-pass-create-invalid-ir{emit-invalid-ir=true signal-pass-failure=true})' -mlir-print-ir-after-failure 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID
+// RUN: not mlir-opt %s -pass-pipeline='func.func(test-pass-create-invalid-ir{emit-invalid-ir=true signal-pass-failure=false})' -mlir-print-ir-after-failure 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID
+// RUN: not mlir-opt %s -pass-pipeline='func.func(test-pass-create-invalid-ir{emit-invalid-ir=false signal-pass-failure=true})' -mlir-print-ir-after-failure 2>&1  | FileCheck %s --check-prefix=CHECK-VALID
+// RUN: mlir-opt %s -pass-pipeline='func.func(test-pass-create-invalid-ir{emit-invalid-ir=false signal-pass-failure=false})' -mlir-print-ir-after-failure 2>&1  | FileCheck %s --check-prefix=CHECK-VALID
+
+// Test whether we print generically or not on pass failure, depending on whether there is invalid IR or not.
+
+// CHECK-VALID: func @TestCreateInvalidCallInPass
+// CHECK-INVALID: "func.func"
+func @TestCreateInvalidCallInPass() {
+  return
+}

diff  --git a/mlir/test/Pass/ir-printing.mlir b/mlir/test/Pass/ir-printing.mlir
index e60b0265f6228..17569b65431db 100644
--- a/mlir/test/Pass/ir-printing.mlir
+++ b/mlir/test/Pass/ir-printing.mlir
@@ -64,3 +64,4 @@ func @bar() {
 
 // AFTER_FAILURE-NOT: // -----// IR Dump After{{.*}}CSE
 // AFTER_FAILURE: // -----// IR Dump After{{.*}}TestFailurePass Failed //----- //
+// AFTER_FAILURE: func @foo()

diff  --git a/mlir/test/lib/Pass/TestPassManager.cpp b/mlir/test/lib/Pass/TestPassManager.cpp
index 68c67703e5942..928d5c11aabe4 100644
--- a/mlir/test/lib/Pass/TestPassManager.cpp
+++ b/mlir/test/lib/Pass/TestPassManager.cpp
@@ -108,6 +108,37 @@ struct TestFailurePass : public PassWrapper<TestFailurePass, OperationPass<>> {
   }
 };
 
+/// A test pass that creates an invalid operation in a function body.
+struct TestInvalidIRPass
+    : public PassWrapper<TestInvalidIRPass,
+                         InterfacePass<FunctionOpInterface>> {
+  MLIR_DEFINE_EXPLICIT_INTERNAL_INLINE_TYPE_ID(TestInvalidIRPass)
+
+  TestInvalidIRPass() = default;
+  TestInvalidIRPass(const TestInvalidIRPass &other) {}
+
+  StringRef getArgument() const final { return "test-pass-create-invalid-ir"; }
+  StringRef getDescription() const final {
+    return "Test pass that adds an invalid operation in a function body";
+  }
+  void getDependentDialects(DialectRegistry &registry) const final {
+    registry.insert<test::TestDialect>();
+  }
+  void runOnOperation() final {
+    if (signalFailure)
+      signalPassFailure();
+    if (!emitInvalidIR)
+      return;
+    OpBuilder b(getOperation().getBody());
+    OperationState state(b.getUnknownLoc(), "test.any_attr_of_i32_str");
+    b.create(state);
+  }
+  Option<bool> signalFailure{*this, "signal-pass-failure",
+                             llvm::cl::desc("Trigger a pass failure")};
+  Option<bool> emitInvalidIR{*this, "emit-invalid-ir", llvm::cl::init(true),
+                             llvm::cl::desc("Emit invalid IR")};
+};
+
 /// A test pass that always fails to enable testing the failure recovery
 /// mechanisms of the pass manager.
 struct TestInvalidParentPass
@@ -179,6 +210,7 @@ void registerPassManagerTestPass() {
 
   PassRegistration<TestCrashRecoveryPass>();
   PassRegistration<TestFailurePass>();
+  PassRegistration<TestInvalidIRPass>();
   PassRegistration<TestInvalidParentPass>();
 
   PassRegistration<TestStatisticPass>();