[Mlir-commits] [mlir] [MLIR] Do not abort on invalid --mlir-debug-counter values (PR #181751)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-core

Author: Shashi Shankar (shashforge)

<details>
<summary>Changes</summary>

Malformed --mlir-debug-counter values currently trigger llvm::report_fatal_error, which results in an LLVM ERROR message and stack dump for invalid user input. Command-line option parsing errors should instead produce a clean diagnostic and exit with a non-zero status, consistent with otherMLIR and LLVM command-line options.

Changes:
- Replace report_fatal_error paths in DebugCounter option parsing with
  cl::Option::error() diagnostics and exit(1).
- Add mlir-opt lit tests covering:
  * invalid numeric value: -1n
  * missing '=' form
  * missing -skip/-count suffix
  * a valid configuration (no regression)

Testing:
- ninja check-mlir
- ./build/bin/llvm-lit -v build/tools/mlir/test/mlir-opt/debugcounter-invalid-cl-options.mlir

Fixes :  #<!-- -->180117

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


2 Files Affected:

- (modified) mlir/lib/Debug/DebugCounter.cpp (+17-16) 
- (added) mlir/test/mlir-opt/debugcounter-invalid-cl-options.mlir (+36) 


``````````diff
diff --git a/mlir/lib/Debug/DebugCounter.cpp b/mlir/lib/Debug/DebugCounter.cpp
index e026a982859a9..879f6c9365cf6 100644
--- a/mlir/lib/Debug/DebugCounter.cpp
+++ b/mlir/lib/Debug/DebugCounter.cpp
@@ -132,22 +132,23 @@ void DebugCounter::applyCLOptions() {
     // Debug counter arguments are expected to be in the form: `counter=value`.
     auto [counterName, counterValueStr] = arg.split('=');
     if (counterValueStr.empty()) {
-      llvm::errs() << "error: expected DebugCounter argument to have an `=` "
-                      "separating the counter name and value, but the provided "
-                      "argument was: `"
-                   << arg << "`\n";
-      llvm::report_fatal_error(
-          "Invalid DebugCounter command-line configuration");
+      clOptions->counters.error(
+          llvm::Twine(
+              "expected DebugCounter argument to have an `=` separating "
+              "the counter name and value, but the provided argument "
+              "was: `") +
+          arg + "`");
+      exit(1);
     }
 
     // Extract the counter value.
     int64_t counterValue;
     if (counterValueStr.getAsInteger(0, counterValue)) {
-      llvm::errs() << "error: expected DebugCounter counter value to be "
-                      "numeric, but got `"
-                   << counterValueStr << "`\n";
-      llvm::report_fatal_error(
-          "Invalid DebugCounter command-line configuration");
+      clOptions->counters.error(
+          llvm::Twine("expected DebugCounter counter value to be numeric, but "
+                      "got `") +
+          counterValueStr + "`");
+      exit(1);
     }
 
     // Now we need to see if this is the skip or the count, remove the suffix,
@@ -159,11 +160,11 @@ void DebugCounter::applyCLOptions() {
       counters[counterName].countToStopAfter = counterValue;
 
     } else {
-      llvm::errs() << "error: expected DebugCounter counter name to end with "
-                      "either `-skip` or `-count`, but got`"
-                   << counterName << "`\n";
-      llvm::report_fatal_error(
-          "Invalid DebugCounter command-line configuration");
+      clOptions->counters.error(
+          llvm::Twine("expected DebugCounter counter name to end with either "
+                      "`-skip` or `-count`, but got `") +
+          counterName + "`");
+      exit(1);
     }
   }
 }
diff --git a/mlir/test/mlir-opt/debugcounter-invalid-cl-options.mlir b/mlir/test/mlir-opt/debugcounter-invalid-cl-options.mlir
new file mode 100644
index 0000000000000..482ef792f13d4
--- /dev/null
+++ b/mlir/test/mlir-opt/debugcounter-invalid-cl-options.mlir
@@ -0,0 +1,36 @@
+// RUN: not mlir-opt %s --mlir-disable-threading \
+// RUN:   --mlir-debug-counter=unique-tag-for-my-action-skip=-1n 2>&1 \
+// RUN: | FileCheck %s --check-prefix=BADNUM
+//
+// RUN: not mlir-opt %s --mlir-disable-threading \
+// RUN:   --mlir-debug-counter=unique-tag-for-my-action-skip 2>&1 \
+// RUN: | FileCheck %s --check-prefix=NOEQ
+//
+// RUN: not mlir-opt %s --mlir-disable-threading \
+// RUN:   --mlir-debug-counter=unique-tag-for-my-action=-1 2>&1 \
+// RUN: | FileCheck %s --check-prefix=BADSFX
+//
+// RUN: mlir-opt %s --mlir-disable-threading \
+// RUN:   --mlir-debug-counter=pass-execution-skip=1 \
+// RUN:   --mlir-print-debug-counter \
+// RUN:   --pass-pipeline="builtin.module(func.func(canonicalize))" 2>&1 \
+// RUN: | FileCheck %s --check-prefix=VALID
+
+func.func @foo() {
+  return
+}
+
+// BADNUM-NOT: LLVM ERROR
+// BADNUM-NOT: Stack dump:
+// BADNUM: {{.*}}: for the {{-+}}mlir-debug-counter option: expected DebugCounter counter value to be numeric, but got `-1n`
+//
+// NOEQ-NOT: LLVM ERROR
+// NOEQ-NOT: Stack dump:
+// NOEQ: {{.*}}: for the {{-+}}mlir-debug-counter option: expected DebugCounter argument to have an `=` separating the counter name and value, but the provided argument was: `unique-tag-for-my-action-skip`
+//
+// BADSFX-NOT: LLVM ERROR
+// BADSFX-NOT: Stack dump:
+// BADSFX: {{.*}}: for the {{-+}}mlir-debug-counter option: expected DebugCounter counter name to end with either `-skip` or `-count`, but got `unique-tag-for-my-action`
+//
+// VALID: DebugCounter counters:
+// VALID: pass-execution : {1,1,-1}

``````````

</details>


https://github.com/llvm/llvm-project/pull/181751