[mlir] implement `-verify-diagnostics=only-expected` #135131

makslevental · 2025-04-10T05:28:34Z

This PR implements verify-diagnostics=only-expected which is a "best effort" verification - i.e., unexpecteds and near-misses will not be considered failures. The purpose is to enable narrowly scoped checking of verification remarks (just as we have for lit where only a subset of lines get CHECKed).

github-actions · 2025-04-10T05:32:36Z

✅ With the latest revision this PR passed the C/C++ code formatter.

llvmbot · 2025-04-10T17:50:20Z

@llvm/pr-subscribers-mlir-core

@llvm/pr-subscribers-mlir

Author: Maksim Levental (makslevental)

Changes

This PR implements verify-diagnostics=only-expected which is a "best effort" verification - i.e., unexpecteds and near-misses will not be considered failures. The purpose is to enable narrowly scoped checking of verification remarks (just as we have for lit where only a subset of lines get CHECKed).

Full diff: https://github.com/llvm/llvm-project/pull/135131.diff

9 Files Affected:

(modified) mlir/examples/transform-opt/mlir-transform-opt.cpp (+31-13)
(modified) mlir/include/mlir/IR/Diagnostics.h (+7-2)
(modified) mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h (+13-3)
(modified) mlir/lib/IR/Diagnostics.cpp (+7-4)
(modified) mlir/lib/Tools/mlir-opt/MlirOptMain.cpp (+22-6)
(modified) mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp (+21-10)
(modified) mlir/test/Pass/full_diagnostics.mlir (+1)
(added) mlir/test/Pass/full_diagnostics_only_expected.mlir (+17)
(added) mlir/test/mlir-translate/verify-only-expected.mlir (+49)

diff --git a/mlir/examples/transform-opt/mlir-transform-opt.cpp b/mlir/examples/transform-opt/mlir-transform-opt.cpp
index 10e16096211ad..ce23e2d67101c 100644
--- a/mlir/examples/transform-opt/mlir-transform-opt.cpp
+++ b/mlir/examples/transform-opt/mlir-transform-opt.cpp
@@ -38,11 +38,20 @@ struct MlirTransformOptCLOptions {
       cl::desc("Allow operations coming from an unregistered dialect"),
       cl::init(false)};
 
-  cl::opt<bool> verifyDiagnostics{
-      "verify-diagnostics",
-      cl::desc("Check that emitted diagnostics match expected-* lines "
-               "on the corresponding line"),
-      cl::init(false)};
+  cl::opt<mlir::SourceMgrDiagnosticVerifierHandler::Level> verifyDiagnostics{
+      "verify-diagnostics", llvm::cl::ValueOptional,
+      cl::desc("Check that emitted diagnostics match "
+               "expected-* lines on the corresponding line"),
+      cl::values(
+          clEnumValN(
+              mlir::SourceMgrDiagnosticVerifierHandler::Level::All, "all",
+              "Check all diagnostics (expected, unexpected, near-misses)"),
+          clEnumValN(
+              mlir::SourceMgrDiagnosticVerifierHandler::Level::All, "",
+              "Check all diagnostics (expected, unexpected, near-misses)"),
+          clEnumValN(
+              mlir::SourceMgrDiagnosticVerifierHandler::Level::OnlyExpected,
+              "only-expected", "Check only expected diagnostics"))};
 
   cl::opt<std::string> payloadFilename{cl::Positional, cl::desc("<input file>"),
                                        cl::init("-")};
@@ -102,12 +111,17 @@ class DiagnosticHandlerWrapper {
 
   /// Constructs the diagnostic handler of the specified kind of the given
   /// source manager and context.
-  DiagnosticHandlerWrapper(Kind kind, llvm::SourceMgr &mgr,
-                           mlir::MLIRContext *context) {
-    if (kind == Kind::EmitDiagnostics)
+  DiagnosticHandlerWrapper(
+      Kind kind, llvm::SourceMgr &mgr, mlir::MLIRContext *context,
+      std::optional<mlir::SourceMgrDiagnosticVerifierHandler::Level> level =
+          {}) {
+    if (kind == Kind::EmitDiagnostics) {
       handler = new mlir::SourceMgrDiagnosticHandler(mgr, context);
-    else
-      handler = new mlir::SourceMgrDiagnosticVerifierHandler(mgr, context);
+    } else {
+      assert(level.has_value() && "expected level");
+      handler =
+          new mlir::SourceMgrDiagnosticVerifierHandler(mgr, context, *level);
+    }
   }
 
   /// This object is non-copyable but movable.
@@ -150,7 +164,9 @@ class TransformSourceMgr {
 public:
   /// Constructs the source manager indicating whether diagnostic messages will
   /// be verified later on.
-  explicit TransformSourceMgr(bool verifyDiagnostics)
+  explicit TransformSourceMgr(
+      std::optional<mlir::SourceMgrDiagnosticVerifierHandler::Level>
+          verifyDiagnostics)
       : verifyDiagnostics(verifyDiagnostics) {}
 
   /// Deconstructs the source manager. Note that `checkResults` must have been
@@ -179,7 +195,8 @@ class TransformSourceMgr {
     // verification needs to happen and store it.
     if (verifyDiagnostics) {
       diagHandlers.emplace_back(
-          DiagnosticHandlerWrapper::Kind::VerifyDiagnostics, mgr, &context);
+          DiagnosticHandlerWrapper::Kind::VerifyDiagnostics, mgr, &context,
+          verifyDiagnostics);
     } else {
       diagHandlers.emplace_back(DiagnosticHandlerWrapper::Kind::EmitDiagnostics,
                                 mgr, &context);
@@ -204,7 +221,8 @@ class TransformSourceMgr {
 
 private:
   /// Indicates whether diagnostic message verification is requested.
-  const bool verifyDiagnostics;
+  const std::optional<mlir::SourceMgrDiagnosticVerifierHandler::Level>
+      verifyDiagnostics;
 
   /// Indicates that diagnostic message verification has taken place, and the
   /// deconstruction is therefore safe.
diff --git a/mlir/include/mlir/IR/Diagnostics.h b/mlir/include/mlir/IR/Diagnostics.h
index 36c433c63b26d..77fa022146759 100644
--- a/mlir/include/mlir/IR/Diagnostics.h
+++ b/mlir/include/mlir/IR/Diagnostics.h
@@ -626,9 +626,12 @@ struct SourceMgrDiagnosticVerifierHandlerImpl;
 /// corresponding line of the source file.
 class SourceMgrDiagnosticVerifierHandler : public SourceMgrDiagnosticHandler {
 public:
+  enum class Level { None = 0, All, OnlyExpected };
   SourceMgrDiagnosticVerifierHandler(llvm::SourceMgr &srcMgr, MLIRContext *ctx,
-                                     raw_ostream &out);
-  SourceMgrDiagnosticVerifierHandler(llvm::SourceMgr &srcMgr, MLIRContext *ctx);
+                                     raw_ostream &out,
+                                     Level level = Level::All);
+  SourceMgrDiagnosticVerifierHandler(llvm::SourceMgr &srcMgr, MLIRContext *ctx,
+                                     Level level = Level::All);
   ~SourceMgrDiagnosticVerifierHandler();
 
   /// Returns the status of the handler and verifies that all expected
@@ -644,6 +647,8 @@ class SourceMgrDiagnosticVerifierHandler : public SourceMgrDiagnosticHandler {
   void process(FileLineColLoc loc, StringRef msg, DiagnosticSeverity kind);
 
   std::unique_ptr<detail::SourceMgrDiagnosticVerifierHandlerImpl> impl;
+
+  Level level = Level::All;
 };
 
 //===----------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
index 09bd86b9581df..ab9395791f179 100644
--- a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
+++ b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
@@ -185,11 +185,20 @@ class MlirOptMainConfig {
 
   /// Set whether to check that emitted diagnostics match `expected-*` lines on
   /// the corresponding line. This is meant for implementing diagnostic tests.
-  MlirOptMainConfig &verifyDiagnostics(bool verify) {
+  MlirOptMainConfig &
+  verifyDiagnostics(SourceMgrDiagnosticVerifierHandler::Level verify) {
     verifyDiagnosticsFlag = verify;
     return *this;
   }
-  bool shouldVerifyDiagnostics() const { return verifyDiagnosticsFlag; }
+
+  bool shouldVerifyDiagnostics() const {
+    return verifyDiagnosticsFlag !=
+           SourceMgrDiagnosticVerifierHandler::Level::None;
+  }
+
+  SourceMgrDiagnosticVerifierHandler::Level verifyDiagnosticsLevel() const {
+    return verifyDiagnosticsFlag;
+  }
 
   /// Set whether to run the verifier after each transformation pass.
   MlirOptMainConfig &verifyPasses(bool verify) {
@@ -279,7 +288,8 @@ class MlirOptMainConfig {
 
   /// Set whether to check that emitted diagnostics match `expected-*` lines on
   /// the corresponding line. This is meant for implementing diagnostic tests.
-  bool verifyDiagnosticsFlag = false;
+  SourceMgrDiagnosticVerifierHandler::Level verifyDiagnosticsFlag =
+      SourceMgrDiagnosticVerifierHandler::Level::None;
 
   /// Run the verifier after each transformation pass.
   bool verifyPassesFlag = true;
diff --git a/mlir/lib/IR/Diagnostics.cpp b/mlir/lib/IR/Diagnostics.cpp
index 19b32120f5890..4ac4bf4a1b37c 100644
--- a/mlir/lib/IR/Diagnostics.cpp
+++ b/mlir/lib/IR/Diagnostics.cpp
@@ -795,9 +795,9 @@ SourceMgrDiagnosticVerifierHandlerImpl::computeExpectedDiags(
 }
 
 SourceMgrDiagnosticVerifierHandler::SourceMgrDiagnosticVerifierHandler(
-    llvm::SourceMgr &srcMgr, MLIRContext *ctx, raw_ostream &out)
+    llvm::SourceMgr &srcMgr, MLIRContext *ctx, raw_ostream &out, Level level)
     : SourceMgrDiagnosticHandler(srcMgr, ctx, out),
-      impl(new SourceMgrDiagnosticVerifierHandlerImpl()) {
+      impl(new SourceMgrDiagnosticVerifierHandlerImpl()), level(level) {
   // Compute the expected diagnostics for each of the current files in the
   // source manager.
   for (unsigned i = 0, e = mgr.getNumBuffers(); i != e; ++i)
@@ -815,8 +815,8 @@ SourceMgrDiagnosticVerifierHandler::SourceMgrDiagnosticVerifierHandler(
 }
 
 SourceMgrDiagnosticVerifierHandler::SourceMgrDiagnosticVerifierHandler(
-    llvm::SourceMgr &srcMgr, MLIRContext *ctx)
-    : SourceMgrDiagnosticVerifierHandler(srcMgr, ctx, llvm::errs()) {}
+    llvm::SourceMgr &srcMgr, MLIRContext *ctx, Level level)
+    : SourceMgrDiagnosticVerifierHandler(srcMgr, ctx, llvm::errs(), level) {}
 
 SourceMgrDiagnosticVerifierHandler::~SourceMgrDiagnosticVerifierHandler() {
   // Ensure that all expected diagnostics were handled.
@@ -886,6 +886,9 @@ void SourceMgrDiagnosticVerifierHandler::process(FileLineColLoc loc,
     }
   }
 
+  if (level == Level::OnlyExpected)
+    return;
+
   // Otherwise, emit an error for the near miss.
   if (nearMiss)
     mgr.PrintMessage(os, nearMiss->fileLoc, llvm::SourceMgr::DK_Error,
diff --git a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
index 9bbf91de18305..2774dfe7e69a1 100644
--- a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
+++ b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
@@ -168,11 +168,26 @@ struct MlirOptMainConfigCLOptions : public MlirOptMainConfig {
         cl::desc("Split marker to use for merging the ouput"),
         cl::location(outputSplitMarkerFlag), cl::init(kDefaultSplitMarker));
 
-    static cl::opt<bool, /*ExternalStorage=*/true> verifyDiagnostics(
-        "verify-diagnostics",
-        cl::desc("Check that emitted diagnostics match "
-                 "expected-* lines on the corresponding line"),
-        cl::location(verifyDiagnosticsFlag), cl::init(false));
+    static cl::opt<SourceMgrDiagnosticVerifierHandler::Level,
+                   /*ExternalStorage=*/true>
+        verifyDiagnostics{
+            "verify-diagnostics", llvm::cl::ValueOptional,
+            cl::desc("Check that emitted diagnostics match "
+                     "expected-* lines on the corresponding line"),
+            cl::location(verifyDiagnosticsFlag),
+            cl::values(
+                clEnumValN(SourceMgrDiagnosticVerifierHandler::Level::All,
+                           "all",
+                           "Check all diagnostics (expected, unexpected, "
+                           "near-misses)"),
+                // Implicit value: when passed with no arguments, e.g.
+                // `--verify-diagnostics` or `--verify-diagnostics=`.
+                clEnumValN(SourceMgrDiagnosticVerifierHandler::Level::All, "",
+                           "Check all diagnostics (expected, unexpected, "
+                           "near-misses)"),
+                clEnumValN(
+                    SourceMgrDiagnosticVerifierHandler::Level::OnlyExpected,
+                    "only-expected", "Check only expected diagnostics"))};
 
     static cl::opt<bool, /*ExternalStorage=*/true> verifyPasses(
         "verify-each",
@@ -542,7 +557,8 @@ static LogicalResult processBuffer(raw_ostream &os,
     return performActions(os, sourceMgr, &context, config);
   }
 
-  SourceMgrDiagnosticVerifierHandler sourceMgrHandler(*sourceMgr, &context);
+  SourceMgrDiagnosticVerifierHandler sourceMgrHandler(
+      *sourceMgr, &context, config.verifyDiagnosticsLevel());
 
   // Do any processing requested by command line flags.  We don't care whether
   // these actions succeed or fail, we only care what diagnostics they produce
diff --git a/mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp b/mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp
index 56773f599d5ce..ceccd1ea69e96 100644
--- a/mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp
+++ b/mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp
@@ -58,7 +58,8 @@ LogicalResult mlir::mlirTranslateMain(int argc, char **argv,
 
   static llvm::cl::opt<bool> allowUnregisteredDialects(
       "allow-unregistered-dialect",
-      llvm::cl::desc("Allow operation with no registered dialects (discouraged: testing only!)"),
+      llvm::cl::desc("Allow operation with no registered dialects "
+                     "(discouraged: testing only!)"),
       llvm::cl::init(false));
 
   static llvm::cl::opt<std::string> inputSplitMarker{
@@ -72,11 +73,21 @@ LogicalResult mlir::mlirTranslateMain(int argc, char **argv,
                      "default marker and process each chunk independently"),
       llvm::cl::init("")};
 
-  static llvm::cl::opt<bool> verifyDiagnostics(
-      "verify-diagnostics",
-      llvm::cl::desc("Check that emitted diagnostics match "
-                     "expected-* lines on the corresponding line"),
-      llvm::cl::init(false));
+  static llvm::cl::opt<SourceMgrDiagnosticVerifierHandler::Level>
+      verifyDiagnostics{
+          "verify-diagnostics", llvm::cl::ValueOptional,
+          llvm::cl::desc("Check that emitted diagnostics match "
+                         "expected-* lines on the corresponding line"),
+          llvm::cl::values(
+              clEnumValN(
+                  SourceMgrDiagnosticVerifierHandler::Level::All, "all",
+                  "Check all diagnostics (expected, unexpected, near-misses)"),
+              clEnumValN(
+                  SourceMgrDiagnosticVerifierHandler::Level::All, "",
+                  "Check all diagnostics (expected, unexpected, near-misses)"),
+              clEnumValN(
+                  SourceMgrDiagnosticVerifierHandler::Level::OnlyExpected,
+                  "only-expected", "Check only expected diagnostics"))};
 
   static llvm::cl::opt<bool> errorDiagnosticsOnly(
       "error-diagnostics-only",
@@ -149,17 +160,17 @@ LogicalResult mlir::mlirTranslateMain(int argc, char **argv,
 
       MLIRContext context;
       context.allowUnregisteredDialects(allowUnregisteredDialects);
-      context.printOpOnDiagnostic(!verifyDiagnostics);
+      context.printOpOnDiagnostic(!bool(verifyDiagnostics.getNumOccurrences()));
       auto sourceMgr = std::make_shared<llvm::SourceMgr>();
       sourceMgr->AddNewSourceBuffer(std::move(ownedBuffer), SMLoc());
 
-      if (verifyDiagnostics) {
+      if (verifyDiagnostics.getNumOccurrences()) {
         // In the diagnostic verification flow, we ignore whether the
         // translation failed (in most cases, it is expected to fail) and we do
         // not filter non-error diagnostics even if `errorDiagnosticsOnly` is
         // set. Instead, we check if the diagnostics were produced as expected.
-        SourceMgrDiagnosticVerifierHandler sourceMgrHandler(*sourceMgr,
-                                                            &context);
+        SourceMgrDiagnosticVerifierHandler sourceMgrHandler(
+            *sourceMgr, &context, verifyDiagnostics);
         (void)(*translationRequested)(sourceMgr, os, &context);
         result = sourceMgrHandler.verify();
       } else if (errorDiagnosticsOnly) {
diff --git a/mlir/test/Pass/full_diagnostics.mlir b/mlir/test/Pass/full_diagnostics.mlir
index 881260ce60d32..c355fd071bdb6 100644
--- a/mlir/test/Pass/full_diagnostics.mlir
+++ b/mlir/test/Pass/full_diagnostics.mlir
@@ -1,4 +1,5 @@
 // RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(test-pass-failure{gen-diagnostics}))' -verify-diagnostics
+// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(test-pass-failure{gen-diagnostics}))' -verify-diagnostics=all
 
 // Test that all errors are reported.
 // expected-error@below {{illegal operation}}
diff --git a/mlir/test/Pass/full_diagnostics_only_expected.mlir b/mlir/test/Pass/full_diagnostics_only_expected.mlir
new file mode 100644
index 0000000000000..de29693604b4f
--- /dev/null
+++ b/mlir/test/Pass/full_diagnostics_only_expected.mlir
@@ -0,0 +1,17 @@
+// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(test-pass-failure{gen-diagnostics}))' -verify-diagnostics=only-expected
+
+// Test that only expected errors are reported.
+// reports {{illegal operation}} but unchecked
+func.func @TestAlwaysIllegalOperationPass1() {
+  return
+}
+
+// expected-error@+1 {{illegal operation}}
+func.func @TestAlwaysIllegalOperationPass2() {
+  return
+}
+
+// reports {{illegal operation}} but unchecked
+func.func @TestAlwaysIllegalOperationPass3() {
+  return
+}
diff --git a/mlir/test/mlir-translate/verify-only-expected.mlir b/mlir/test/mlir-translate/verify-only-expected.mlir
new file mode 100644
index 0000000000000..543a6954c840f
--- /dev/null
+++ b/mlir/test/mlir-translate/verify-only-expected.mlir
@@ -0,0 +1,49 @@
+// Note: borrowed/copied from mlir/test/Target/LLVMIR/arm-sme-invalid.mlir
+
+// Check that verify-diagnostics=only-expected passes with only one actual `expected-error`
+// RUN: mlir-translate %s -verify-diagnostics=only-expected -split-input-file -mlir-to-llvmir
+
+// Check that verify-diagnostics=all fails because we're missing three `expected-error`
+// RUN: not mlir-translate %s -verify-diagnostics=all -split-input-file -mlir-to-llvmir 2>&1 | FileCheck %s --check-prefix=CHECK-VERIFY-ALL
+// CHECK-VERIFY-ALL:      error: unexpected error: 'arm_sme.intr.write.horiz' op failed to verify that all of {predicate, vector} have same shape
+// CHECK-VERIFY-ALL-NEXT: "arm_sme.intr.write.horiz"
+// CHECK-VERIFY-ALL:      error: unexpected error: 'arm_sme.intr.read.horiz' op failed to verify that all of {vector, res} have same element type
+// CHECK-VERIFY-ALL-NEXT: %res = "arm_sme.intr.read.horiz"
+// CHECK-VERIFY-ALL:      error: unexpected error: 'arm_sme.intr.cntsb' op failed to verify that `res` is i64
+// CHECK-VERIFY-ALL-NEXT: %res = "arm_sme.intr.cntsb"
+
+llvm.func @arm_sme_vector_to_tile_invalid_types(%tileslice : i32,
+                                                %nxv4i1 : vector<[4]xi1>,
+                                                %nxv16i8 : vector<[16]xi8>) {
+  "arm_sme.intr.write.horiz"(%tileslice, %nxv4i1, %nxv16i8) <{tile_id = 0 : i32}> :
+      (i32, vector<[4]xi1>, vector<[16]xi8>) -> ()
+  llvm.return
+}
+
+// -----
+
+llvm.func @arm_sme_tile_slice_to_vector_invalid_shapes(
+  %tileslice : i32, %nxv4i1 : vector<[4]xi1>, %nxv16i8 : vector<[16]xi8>
+) -> vector<[3]xf32> {
+  // expected-error @+1 {{failed to verify that all of {vector, predicate, res} have same shape}}
+  %res = "arm_sme.intr.read.horiz"(%nxv16i8, %nxv4i1, %tileslice) <{tile_id = 0 : i32}> :
+      (vector<[16]xi8>, vector<[4]xi1>, i32) -> vector<[3]xf32>
+  llvm.return %res : vector<[3]xf32>
+}
+
+// -----
+
+llvm.func @arm_sme_tile_slice_to_vector_invalid_element_types(
+  %tileslice : i32, %nxv4i1 : vector<[4]xi1>, %nxv4f32 : vector<[4]xf32>
+) -> vector<[3]xi32> {
+  %res = "arm_sme.intr.read.horiz"(%nxv4f32, %nxv4i1, %tileslice) <{tile_id = 0 : i32}> :
+      (vector<[4]xf32>, vector<[4]xi1>, i32) -> vector<[4]xi32>
+  llvm.return %res : vector<[4]xi32>
+}
+
+// -----
+
+llvm.func @arm_sme_streaming_vl_invalid_return_type() -> i32 {
+  %res = "arm_sme.intr.cntsb"() : () -> i32
+  llvm.return %res : i32
+}

jpienaar

I can see the analogue with FileCheck behavior. It does enable more reduced test cases. It does also expose one to accidentally skipping checking, but given how this used for error/analysis checking often, I don't have much concern as those are targetted.

jpienaar · 2025-04-10T17:51:51Z

mlir/examples/transform-opt/mlir-transform-opt.cpp

-      cl::init(false)};
+  cl::opt<mlir::SourceMgrDiagnosticVerifierHandler::Level> verifyDiagnostics{
+      "verify-diagnostics", llvm::cl::ValueOptional,
+      cl::desc("Check that emitted diagnostics match "


This line break feels off/premature.

clang-format but i can restore it by hand

jpienaar · 2025-04-10T17:52:35Z

mlir/examples/transform-opt/mlir-transform-opt.cpp

+          clEnumValN(
+              mlir::SourceMgrDiagnosticVerifierHandler::Level::All, "all",
+              "Check all diagnostics (expected, unexpected, near-misses)"),
+          clEnumValN(


Could you add a comment that you are repeating enum all for backward compatibility?

Oh this is probably better/more relevant in the non-example one.

jpienaar · 2025-04-10T17:55:19Z

mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp

@@ -58,7 +58,8 @@ LogicalResult mlir::mlirTranslateMain(int argc, char **argv,

  static llvm::cl::opt<bool> allowUnregisteredDialects(
      "allow-unregistered-dialect",
-      llvm::cl::desc("Allow operation with no registered dialects (discouraged: testing only!)"),
+      llvm::cl::desc("Allow operation with no registered dialects "


Unrelated change?

clang-format - i'll undo

jpienaar · 2025-04-10T17:56:01Z

mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp

@@ -149,17 +160,17 @@ LogicalResult mlir::mlirTranslateMain(int argc, char **argv,

      MLIRContext context;
      context.allowUnregisteredDialects(allowUnregisteredDialects);
-      context.printOpOnDiagnostic(!verifyDiagnostics);
+      context.printOpOnDiagnostic(!bool(verifyDiagnostics.getNumOccurrences()));


How about just checking if 0?

potato potato :) - sure

jpienaar · 2025-04-10T17:57:15Z

mlir/test/mlir-translate/verify-only-expected.mlir

It feels like your other test covers this. What does this add additional?

this tests the logic for how the option is actually passed to SourceMgrDiagnosticVerifierHandler, which is slightly different between mlir-translate and mlir-opt

So that also explains why you had to use one of the existing tests? I was going to say it'd be nice to have something more generic, but maybe that's difficult with mlir-translate?

fixed (made the test more "generic")

River707 · 2025-04-10T18:07:19Z

mlir/include/mlir/IR/Diagnostics.h

@@ -644,6 +647,8 @@ class SourceMgrDiagnosticVerifierHandler : public SourceMgrDiagnosticHandler {
  void process(FileLineColLoc loc, StringRef msg, DiagnosticSeverity kind);

  std::unique_ptr<detail::SourceMgrDiagnosticVerifierHandlerImpl> impl;
+
+  Level level = Level::All;


Please move this to the impl struct

you can't? it's an impl struct and so the defn isn't visible outside of Diagnostics.cpp?

In those cases expose a getter method for the Level. We shouldn't intermix pImpl and non-pimpl.

@River707 moved to impl - can you double check that what i did is indeed what you had in mind

River707 · 2025-04-10T18:12:15Z

mlir/test/mlir-translate/verify-only-expected.mlir

So that also explains why you had to use one of the existing tests? I was going to say it'd be nice to have something more generic, but maybe that's difficult with mlir-translate?

Pick up the following changes: * llvm/llvm-project#135243 * llvm/llvm-project#135131

This PR implements `verify-diagnostics=only-expected` which is a "best effort" verification - i.e., `unexpected`s and `near-misses` will not be considered failures. The purpose is to enable narrowly scoped checking of verification remarks (just as we have for lit where only a subset of lines get `CHECK`ed).

Pick up the following changes: * llvm/llvm-project#135243 * llvm/llvm-project#135131

makslevental force-pushed the makslevental/verify-only-specified branch from a960804 to 6d162b1 Compare April 10, 2025 05:30

makslevental force-pushed the makslevental/verify-only-specified branch from 6d162b1 to 2d1ff44 Compare April 10, 2025 07:15

makslevental changed the title ~~[mlir] implement verify-only-specified-diagnostics~~ [mlir] implement verify-only-expected-diagnostics Apr 10, 2025

makslevental force-pushed the makslevental/verify-only-specified branch 2 times, most recently from dc04ddf to af3fbaa Compare April 10, 2025 15:29

makslevental changed the title ~~[mlir] implement verify-only-expected-diagnostics~~ [mlir] implement -verify-diagnostics=only-expected Apr 10, 2025

makslevental requested a review from jpienaar April 10, 2025 17:47

makslevental marked this pull request as ready for review April 10, 2025 17:49

llvmbot added mlir:core MLIR Core Infrastructure mlir labels Apr 10, 2025

jpienaar approved these changes Apr 10, 2025

View reviewed changes

River707 reviewed Apr 10, 2025

View reviewed changes

River707 approved these changes Apr 10, 2025

View reviewed changes

makslevental added 5 commits April 10, 2025 14:33

[mlir] implement verify-only-specified-diagnostics

09671dc

use enum for verifyDiagnostics

2846d1c

address comments

67df339

more generic test

03c5c6b

move level to impl

2e89435

makslevental force-pushed the makslevental/verify-only-specified branch from 8652ec5 to 2e89435 Compare April 10, 2025 18:34

makslevental requested a review from River707 April 10, 2025 18:34

fix mlir-transform-opt

59d69c9

makslevental merged commit 1cec5ff into llvm:main Apr 10, 2025
11 checks passed

makslevental deleted the makslevental/verify-only-specified branch April 10, 2025 22:50

makslevental mentioned this pull request Apr 11, 2025

[Backend] bump to llvm/llvm-project@1cec5fffd8fd triton-lang/triton#6458

Merged

ThomasRaoux pushed a commit to triton-lang/triton that referenced this pull request Apr 11, 2025

[Backend] bump to llvm/llvm-project@1cec5fffd8fd (#6458)

db1147b

Pick up the following changes: * llvm/llvm-project#135243 * llvm/llvm-project#135131

njriasan pushed a commit to njriasan/triton that referenced this pull request Apr 18, 2025

[Backend] bump to llvm/llvm-project@1cec5fffd8fd (triton-lang#6458)

e447301

Pick up the following changes: * llvm/llvm-project#135243 * llvm/llvm-project#135131

[mlir] implement -verify-diagnostics=only-expected #135131

[mlir] implement -verify-diagnostics=only-expected #135131

Uh oh!

Conversation

makslevental commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jpienaar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

[mlir] implement `-verify-diagnostics=only-expected` #135131

[mlir] implement `-verify-diagnostics=only-expected` #135131

makslevental commented Apr 10, 2025 •

edited

Loading

github-actions bot commented Apr 10, 2025 •

edited

Loading

llvmbot commented Apr 10, 2025 •

edited

Loading