Skip to content

[UTC] Don't leave dangling CHECK-SAME when removing CHECK lines #82569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 29, 2024

Conversation

hnrklssn
Copy link
Member

When removing only lines that are global value CHECK lines, a related CHECK-SAME line could be left dangling without a previous line to belong to.

Resolves #78517

When removing only lines that are global value CHECK lines, a related
CHECK-SAME line could be left dangling without a previous line to belong
to.

Resolves llvm#78517
@llvmbot
Copy link
Member

llvmbot commented Feb 22, 2024

@llvm/pr-subscribers-testing-tools

Author: Henrik G. Olsson (hnrklssn)

Changes

When removing only lines that are global value CHECK lines, a related CHECK-SAME line could be left dangling without a previous line to belong to.

Resolves #78517


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

5 Files Affected:

  • (added) llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/global_remove_same.ll (+15)
  • (added) llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/global_remove_same.ll.expected (+13)
  • (added) llvm/test/tools/UpdateTestChecks/update_test_checks/global_remove_same.test (+4)
  • (modified) llvm/utils/UpdateTestChecks/common.py (+17-6)
  • (modified) llvm/utils/update_test_checks.py (+12-3)
diff --git a/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/global_remove_same.ll b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/global_remove_same.ll
new file mode 100644
index 00000000000000..d3d13ae2622e6f
--- /dev/null
+++ b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/global_remove_same.ll
@@ -0,0 +1,15 @@
+; RUN: opt -S < %s | FileCheck %s
+
+define i32 @foo() {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:    [[RESULT:%.*]] = call i32 @bar(i32 0, i32 1)
+; CHECK-NEXT:    ret i32 [[RESULT]]
+;
+  %result = call i32 @bar(i32 0, i32 1)
+  ret i32 %result
+}
+
+declare i32 @bar(i32, i32)
+; CHECK-LABEL: @bar(
+; CHECK-SAME: i32
+; CHECK-SAME: i32
diff --git a/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/global_remove_same.ll.expected b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/global_remove_same.ll.expected
new file mode 100644
index 00000000000000..e76efaedd172c1
--- /dev/null
+++ b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/global_remove_same.ll.expected
@@ -0,0 +1,13 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S < %s | FileCheck %s
+
+define i32 @foo() {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:    [[RESULT:%.*]] = call i32 @bar(i32 0, i32 1)
+; CHECK-NEXT:    ret i32 [[RESULT]]
+;
+  %result = call i32 @bar(i32 0, i32 1)
+  ret i32 %result
+}
+
+declare i32 @bar(i32, i32)
diff --git a/llvm/test/tools/UpdateTestChecks/update_test_checks/global_remove_same.test b/llvm/test/tools/UpdateTestChecks/update_test_checks/global_remove_same.test
new file mode 100644
index 00000000000000..5d447babddea4f
--- /dev/null
+++ b/llvm/test/tools/UpdateTestChecks/update_test_checks/global_remove_same.test
@@ -0,0 +1,4 @@
+## Basic test checking global checks split over multiple lines are removed together
+# RUN: cp -f %S/Inputs/global_remove_same.ll %t.ll && %update_test_checks %t.ll
+# RUN: diff -u %t.ll %S/Inputs/global_remove_same.ll.expected
+
diff --git a/llvm/utils/UpdateTestChecks/common.py b/llvm/utils/UpdateTestChecks/common.py
index 4a02a92f824e65..831dd704160cd1 100644
--- a/llvm/utils/UpdateTestChecks/common.py
+++ b/llvm/utils/UpdateTestChecks/common.py
@@ -387,27 +387,37 @@ def itertests(
             )
 
 
+# Returns a tuple of two bools, where the first value indicates whether the line should be added to the output, and the second indicates whether the line is a CHECK line
 def should_add_line_to_output(
-    input_line, prefix_set, skip_global_checks=False, comment_marker=";"
+    input_line,
+    prefix_set,
+    skip_global_checks=False,
+    skip_same_checks=False,
+    comment_marker=";",
 ):
     # Skip any blank comment lines in the IR.
     if not skip_global_checks and input_line.strip() == comment_marker:
-        return False
+        return False, False
     # Skip a special double comment line we use as a separator.
     if input_line.strip() == comment_marker + SEPARATOR:
-        return False
+        return False, False
     # Skip any blank lines in the IR.
     # if input_line.strip() == '':
     #  return False
     # And skip any CHECK lines. We're building our own.
     m = CHECK_RE.match(input_line)
     if m and m.group(1) in prefix_set:
+        if skip_same_checks and CHECK_SAME_RE.match(input_line):
+            # The previous CHECK line was removed, so don't leave this dangling
+            return False, True
         if skip_global_checks:
+            # Skip checks only if they are of global value definitions
             global_ir_value_re = re.compile(r"(\[\[|@)", flags=(re.M))
-            return not global_ir_value_re.search(input_line)
-        return False
+            is_global = global_ir_value_re.search(input_line)
+            return not is_global, True
+        return False, True
 
-    return True
+    return True, False
 
 
 # Perform lit-like substitutions
@@ -483,6 +493,7 @@ def invoke_tool(exe, cmd_args, ir, preprocess_cmd=None, verbose=False):
 CHECK_RE = re.compile(
     r"^\s*(?://|[;#])\s*([^:]+?)(?:-NEXT|-NOT|-DAG|-LABEL|-SAME|-EMPTY)?:"
 )
+CHECK_SAME_RE = re.compile(r"^\s*(?://|[;#])\s*([^:]+?)(?:-SAME)?:")
 
 UTC_ARGS_KEY = "UTC_ARGS:"
 UTC_ARGS_CMD = re.compile(r".*" + UTC_ARGS_KEY + r"\s*(?P<cmd>.*)\s*$")
diff --git a/llvm/utils/update_test_checks.py b/llvm/utils/update_test_checks.py
index 06c247c8010a90..451f76389a3560 100755
--- a/llvm/utils/update_test_checks.py
+++ b/llvm/utils/update_test_checks.py
@@ -235,6 +235,7 @@ def main():
             )
         else:
             # "Normal" mode.
+            skip_same_check_line = False
             for input_line_info in ti.iterlines(output_lines):
                 input_line = input_line_info.line
                 args = input_line_info.args
@@ -281,18 +282,26 @@ def main():
                         )
                     has_checked_pre_function_globals = True
 
-                if common.should_add_line_to_output(
-                    input_line, prefix_set, not is_in_function
-                ):
+                should_add, is_check_line = common.should_add_line_to_output(
+                    input_line,
+                    prefix_set,
+                    not is_in_function,
+                    skip_same_check_line,
+                )
+                if should_add:
                     # This input line of the function body will go as-is into the output.
                     # Except make leading whitespace uniform: 2 spaces.
                     input_line = common.SCRUB_LEADING_WHITESPACE_RE.sub(
                         r"  ", input_line
                     )
                     output_lines.append(input_line)
+                    skip_same_check_line = False
                     if input_line.strip() == "}":
                         is_in_function = False
                         continue
+                else:
+                    # If we are removing a check line, and the next line is CHECK-SAME, it MUST also be removed
+                    skip_same_check_line = is_check_line
 
                 if is_in_function:
                     continue

Copy link
Member

@arichardson arichardson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM assuming all tests are passing (do the other update scripts need changes too, e.g. update_cc_test_checks.py?)

@hnrklssn
Copy link
Member Author

LGTM assuming all tests are passing (do the other update scripts need changes too, e.g. update_cc_test_checks.py?)

No, doesn't seem like it, luckily

@hnrklssn hnrklssn merged commit 6a65b44 into llvm:main Feb 29, 2024
mylai-mtk pushed a commit to mylai-mtk/llvm-project that referenced this pull request Jul 12, 2024
…#82569)

When removing only lines that are global value CHECK lines, a related
CHECK-SAME line could be left dangling without a previous line to belong
to.

Resolves llvm#78517
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[UTC] Update test check removes check lines for declarations (and leaves the argument checks)
3 participants