[utils] Use stricter SSA regexp for CHECK-SAME. #128083
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When CHECK-SAME checks are split across multiple lines, the '.*' regexp for the SSA variable name may cause problems, e.g.: // CHECK_LABEL: func.func @whatever( // CHECK-SAME: %[[VAL_0:.*]]: i32, // CHECK-SAME: %[[VAL_1:.*]]: i32, // CHECK-SAME: %[[VAL_2:.*]]: i64) This will not work for `func.func @whatever(%0: i32, %1: i32, %2: i64)`, because VAL_0 will match to `0: i32, %1`.
|
@llvm/pr-subscribers-mlir Author: Slava Zakharin (vzakhari) ChangesWhen CHECK-SAME checks are split across multiple lines, This will not work for Full diff: https://github.com/llvm/llvm-project/pull/128083.diff 1 Files Affected:
diff --git a/mlir/utils/generate-test-checks.py b/mlir/utils/generate-test-checks.py
index 8faa425beace1..80069737be97d 100755
--- a/mlir/utils/generate-test-checks.py
+++ b/mlir/utils/generate-test-checks.py
@@ -159,7 +159,7 @@ def get_num_ssa_results(input_line):
# Process a line of input that has been split at each SSA identifier '%'.
-def process_line(line_chunks, variable_namer):
+def process_line(line_chunks, variable_namer, strict=False):
output_line = ""
# Process the rest that contained an SSA value name.
@@ -180,7 +180,14 @@ def process_line(line_chunks, variable_namer):
else:
# Otherwise, generate a new variable.
variable = variable_namer.generate_name(ssa_name)
- output_line += "%[[" + variable + ":.*]]"
+ if strict:
+ # Use stricter regexp for the variable name, if requested.
+ # Greedy matching may cause issues with the generic '.*'
+ # regexp when the checks are split across several
+ # lines (e.g. for CHECK-SAME).
+ output_line += "%[[" + variable + ":" + SSA_RE_STR + "]]"
+ else:
+ output_line += "%[[" + variable + ":.*]]"
# Append the non named group.
output_line += chunk[len(ssa_name) :]
@@ -390,7 +397,7 @@ def main():
output_line += " " * len(ssa_split[0])
# Process the rest of the line.
- output_line += process_line([argument], variable_namer)
+ output_line += process_line([argument], variable_namer, strict=True)
# Append the output line.
output_segments[-1].append(output_line)
|
River707
approved these changes
Feb 21, 2025
mlir/utils/generate-test-checks.py
Outdated
| @@ -180,7 +180,14 @@ def process_line(line_chunks, variable_namer): | |||
| else: | |||
| # Otherwise, generate a new variable. | |||
| variable = variable_namer.generate_name(ssa_name) | |||
| output_line += "%[[" + variable + ":.*]]" | |||
| if strict: | |||
There was a problem hiding this comment.
Can you use a more descriptive name here? Something like strict_name_re, just to be a bit more clear.
|
✅ With the latest revision this PR passed the Python code formatter. |
|
It looks like I have some old clang-format in my path, so I keep catching formatting issues only on pre-commit. Sorry for the noise... |
banach-space
added a commit
to banach-space/llvm-project
that referenced
this pull request
Apr 22, 2025
Following llvm#128083, all `CHECK-SAME` lines generated by generate-test-checks.py use a strict regex: ```mlir // 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: ```mlir // 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: ```bash generate-test-checks.py --strict_name_re=true file.mlir ```
banach-space
added a commit
that referenced
this pull request
Apr 22, 2025
Following #128083, all `CHECK-SAME` lines generated by generate-test-checks.py use a strict regex: ```mlir // 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: ```mlir // 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: ```bash generate-test-checks.py --strict_name_re=true file.mlir ```
This was referenced Apr 23, 2025
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
Following llvm#128083, all `CHECK-SAME` lines generated by generate-test-checks.py use a strict regex: ```mlir // 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: ```mlir // 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: ```bash generate-test-checks.py --strict_name_re=true file.mlir ```
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
Following llvm#128083, all `CHECK-SAME` lines generated by generate-test-checks.py use a strict regex: ```mlir // 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: ```mlir // 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: ```bash generate-test-checks.py --strict_name_re=true file.mlir ```
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
Following llvm#128083, all `CHECK-SAME` lines generated by generate-test-checks.py use a strict regex: ```mlir // 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: ```mlir // 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: ```bash generate-test-checks.py --strict_name_re=true file.mlir ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When CHECK-SAME checks are split across multiple lines,
the '.*' regexp for the SSA variable name may cause problems, e.g.:
This will not work for
func.func @whatever(%0: i32, %1: i32, %2: i64),because VAL_0 will match to
0: i32, %1.