[mlir][utils] Update generate-test-checks.py

[banach-space]

Following #128083, all CHECK-SAME lines generated by
generate-test-checks.py use a strict regex:

// CHECK-SAME:  %[[VAL_0:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]]: memref<128x256x512xf32>,

However, in most cases this strict form is unnecessary and can obscure
readability. In such cases, the following would be sufficient:

// CHECK-SAME:  %[[VAL_0:.*]]: memref<128x256x512xf32>,

This patch adds a command-line flag to make the strict mode optional. To
enable strict regex matching, use:

generate-test-checks.py --strict_name_re=true file.mlir

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Andrzej Warzyński (banach-space)

Changes

Following #128083, all CHECK-SAME lines generated by
generate-test-checks.py use a strict regex:

// CHECK-SAME:      %[[A:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]] = arith.constant 0 : index

However, in most cases this strict form is unnecessary and can obscure
readability. In such cases, the following would be sufficient:

// CHECK-SAME:      %[[A:.*]] = arith.constant 0 : index

This patch adds a command-line flag to make the strict mode optional. To
enable strict regex matching, use:

generate-test-checks.py --strict_name_re=true file.mlir

---

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

1 Files Affected:

• (modified) mlir/utils/generate-test-checks.py (+7-1)

diff --git a/mlir/utils/generate-test-checks.py b/mlir/utils/generate-test-checks.py
index 394ef7e0f7da0..5e13e61865df3 100755
--- a/mlir/utils/generate-test-checks.py
+++ b/mlir/utils/generate-test-checks.py
@@ -295,6 +295,12 @@ def main():
         help="Names to be used in FileCheck regular expression to represent "
         "attributes in the order they are defined. Separate names with commas,"
         "commas, and leave empty entries for default names (e.g.: 'MAP0,,,MAP1')")
+    parser.add_argument(
+        "--strict_name_re",
+        type=bool,
+        default=False,
+        help="Set to true to use stricter regex for CHECK-SAME directives. "
+        "Use when Greedy matching causes issues with the generic '.*'")
 
     args = parser.parse_args()
 
@@ -406,7 +412,7 @@ def main():
 
                 # Process the rest of the line.
                 output_line += process_line(
-                    [argument], variable_namer, strict_name_re=True
+                    [argument], variable_namer, args.strict_name_re
                 )
 
         # Append the output line.

[banach-space]

@vzakhari , I assumed that in #128083 you were meant to make the "strict" regex optional. If not, I can flip the default in this PR, though personally I'd prefer for us to restore the original default. Let me know what you think, thanks!

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[vzakhari]

Thank you for the change! It makes sense to have an option. I am okay with either default value.

Can you please show an example where such checks may be generated?

// CHECK-SAME:      %[[A:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]] = arith.constant 0 : index

[banach-space]

Thank you for the change! It makes sense to have an option. I am okay with either default value.

Can you please show an example where such checks may be generated?

// CHECK-SAME:      %[[A:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]] = arith.constant 0 : index

Try running the script on this example:

  module {
    func.func @func(%arg0: memref<128x256x512xf32>, %arg1: memref<128x512x256xf32>, %arg2: memref<256x256xf32>) {
      return
    }
  }

I get this:

// CHECK-LABEL:     func.func @func(
// CHECK-SAME:                      %[[VAL_0:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]]: memref<128x256x512xf32>,
// CHECK-SAME:                      %[[VAL_1:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]]: memref<128x512x256xf32>,
// CHECK-SAME:                      %[[VAL_2:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]]: memref<256x256xf32>) {
// CHECK:             return
// CHECK:           }

There's also quite a few tests in this PR #130944 (that I find a bit obscured to the the strict regex).

Btw, I've hit that issue with CHECK-SAME multiple times and really appreciate your fix 🙏🏻

[vzakhari]

Oh, okay :) yes, it is supposed to be applied mostly to the argument lists, I was just confused by the PR comment saying that it can generate something like this for SSA assignment, which would mean there is MLIR with something prepending the SSA assignment.

LGTM

[banach-space]

it is supposed to be applied mostly to the argument lists, I was just confused by the PR comment saying that it can generate something like this for SSA assignment

I updated the summary. That example was generated using a file without an explicit module Op, so the script got confused.

Thanks for the review!