[mlir] generate-test-checks.py does not remove blank lines

[michaelmaitland]

When we generate test checks into the original test file (inplace), it was incorrectly removing empty lines.

I tested this by running

my-downstream-opt-tool file.mlir -mypass | python3 ~/llvm-project/mlir/utils/generate-test-checks.py --source file.mlir -i

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Michael Maitland (michaelmaitland)

Changes

When we generate test checks into the original test file (inplace), it was incorrectly removing empty lines.

I tested this by running

my-downstream-opt-tool file.mlir -mypass | python3 ~/llvm-project/mlir/utils/generate-test-checks.py --source file.mlir -i

---

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

1 Files Affected:

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

diff --git a/mlir/utils/generate-test-checks.py b/mlir/utils/generate-test-checks.py
index 3712a6b9c963d..aae0b14965cad 100755
--- a/mlir/utils/generate-test-checks.py
+++ b/mlir/utils/generate-test-checks.py
@@ -234,10 +234,17 @@ def process_source_lines(source_lines, note, args):
     source_split_re = re.compile(args.source_delim_regex)
 
     source_segments = [[]]
+    skip_next_empty_line = False
     for line in source_lines:
         # Remove previous note.
-        if line in note:
+        if line and line in note:
+            skip_next_empty_line = True
             continue
+        # Skip the first empty line after the note
+        if skip_next_empty_line and not line:
+            skip_next_empty_line = False
+            continue
+        skip_next_empty_line = False
         # Remove previous CHECK lines.
         if line.find(args.check_prefix) != -1:
             continue

[joker-eph]

I think you're trying to patch a logic that was implemented in the wrong place originally. I would instead suggest this patch:

diff --git a/mlir/utils/generate-test-checks.py b/mlir/utils/generate-test-checks.py
index 3712a6b9c963..22774468bb40 100755
--- a/mlir/utils/generate-test-checks.py
+++ b/mlir/utils/generate-test-checks.py
@@ -230,14 +230,11 @@ def process_line(line_chunks, variable_namer, use_ssa_name=False, strict_name_re
 
 
 # Process the source file lines. The source file doesn't have to be .mlir.
-def process_source_lines(source_lines, note, args):
+def process_source_lines(source_lines, args):
     source_split_re = re.compile(args.source_delim_regex)
 
     source_segments = [[]]
     for line in source_lines:
-        # Remove previous note.
-        if line in note:
-            continue
         # Remove previous CHECK lines.
         if line.find(args.check_prefix) != -1:
             continue
@@ -359,9 +356,10 @@ def main():
 
     source_segments = None
     if args.source:
-        source_segments = process_source_lines(
-            [l.rstrip() for l in open(args.source, "r")], autogenerated_note, args
-        )
+        with open(args.source, "r") as f:
+            raw_source = f.read().replace(autogenerated_note, "")
+            raw_source_lines = [l.rstrip() for l in raw_source.splitlines()]
+        source_segments = process_source_lines(raw_source_lines, args)
 
     if args.inplace:
         assert args.output is None

[michaelmaitland]

I think you're trying to patch a logic that was implemented in the wrong place originally. I would instead suggest this patch:

diff --git a/mlir/utils/generate-test-checks.py b/mlir/utils/generate-test-checks.py
index 3712a6b9c963..22774468bb40 100755
--- a/mlir/utils/generate-test-checks.py
+++ b/mlir/utils/generate-test-checks.py
@@ -230,14 +230,11 @@ def process_line(line_chunks, variable_namer, use_ssa_name=False, strict_name_re
 
 
 # Process the source file lines. The source file doesn't have to be .mlir.
-def process_source_lines(source_lines, note, args):
+def process_source_lines(source_lines, args):
     source_split_re = re.compile(args.source_delim_regex)
 
     source_segments = [[]]
     for line in source_lines:
-        # Remove previous note.
-        if line in note:
-            continue
         # Remove previous CHECK lines.
         if line.find(args.check_prefix) != -1:
             continue
@@ -359,9 +356,10 @@ def main():
 
     source_segments = None
     if args.source:
-        source_segments = process_source_lines(
-            [l.rstrip() for l in open(args.source, "r")], autogenerated_note, args
-        )
+        with open(args.source, "r") as f:
+            raw_source = f.read().replace(autogenerated_note, "")
+            raw_source_lines = [l.rstrip() for l in raw_source.splitlines()]
+        source_segments = process_source_lines(raw_source_lines, args)
 
     if args.inplace:
         assert args.output is None

Im more than happy to take your approach. do you want to make a PR?

[joker-eph]

Sure, here you go: #166493