[UTC] Recognise CHECK lines with globals matched literally #70050
Conversation
This file contains 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
Previously when using `-p` a.k.a. `--preserve-names` existing lines for checking globals were not recognised as such, leading to the line being kept while also being emitted again, resulting in duplicated CHECK lines.
|
@llvm/pr-subscribers-testing-tools Author: Henrik G. Olsson (hnrklssn) ChangesPreviously when using This resolves #70048. Full diff: https://github.com/llvm/llvm-project/pull/70050.diff 4 Files Affected:
diff --git a/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/global_preserve_name.ll b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/global_preserve_name.ll
new file mode 100644
index 000000000000000..b872e9d53e2cba1
--- /dev/null
+++ b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/global_preserve_name.ll
@@ -0,0 +1,13 @@
+; RUN: opt -S < %s | FileCheck %s
+
+@G = constant i32 42
+
+;.
+; CHECK: @G = constant i32 42
+;.
+define ptr @foo() {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT: ret ptr @G
+;
+ ret ptr @G
+}
diff --git a/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/global_preserve_name.ll.expected b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/global_preserve_name.ll.expected
new file mode 100644
index 000000000000000..f29ed24be3fec7c
--- /dev/null
+++ b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/global_preserve_name.ll.expected
@@ -0,0 +1,14 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: -p --check-globals
+; RUN: opt -S < %s | FileCheck %s
+
+@G = constant i32 42
+
+;.
+; CHECK: @G = constant i32 42
+;.
+define ptr @foo() {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT: ret ptr @G
+;
+ ret ptr @G
+}
diff --git a/llvm/test/tools/UpdateTestChecks/update_test_checks/global_preserve_name.test b/llvm/test/tools/UpdateTestChecks/update_test_checks/global_preserve_name.test
new file mode 100644
index 000000000000000..824c221f1ad78d6
--- /dev/null
+++ b/llvm/test/tools/UpdateTestChecks/update_test_checks/global_preserve_name.test
@@ -0,0 +1,7 @@
+## Basic test checking that update_test_checks.py works correctly
+# RUN: cp -f %S/Inputs/global_preserve_name.ll %t.ll && %update_test_checks %t.ll --check-globals --preserve-names
+# RUN: diff -u %t.ll %S/Inputs/global_preserve_name.ll.expected
+## Verify that running without the --global-value-regex flag respects UTC_ARGS
+# RUN: %update_test_checks %t.ll
+# RUN: diff -u %t.ll %S/Inputs/global_preserve_name.ll.expected
+
diff --git a/llvm/utils/UpdateTestChecks/common.py b/llvm/utils/UpdateTestChecks/common.py
index c8c8d85e0dc68c8..33da7b3b8665dd5 100644
--- a/llvm/utils/UpdateTestChecks/common.py
+++ b/llvm/utils/UpdateTestChecks/common.py
@@ -391,7 +391,7 @@ def should_add_line_to_output(
m = CHECK_RE.match(input_line)
if m and m.group(1) in prefix_set:
if skip_global_checks:
- global_ir_value_re = re.compile(r"\[\[", flags=(re.M))
+ global_ir_value_re = re.compile(r"(\[\[|@)", flags=(re.M))
return not global_ir_value_re.search(input_line)
return False
|
|
The failures seem unrelated. |
hnrklssn
added a commit
that referenced
this pull request
Nov 13, 2023
Recommits the changes from https://reviews.llvm.org/D148216. Explicitly named globals are now matched literally, instead of emitting a capture group for the name. This resolves #70047. Metadata and annotations, on the other hand, are captured and matched against by default, since their identifiers are not stable. The reasons for revert (#63746) have been fixed: The first issue, that of duplicated checkers, has already been resolved in #70050. This PR resolves the second issue listed in #63746, regarding the order of named and unnamed globals. This is fixed by recording the index of substrings containing global values, and sorting the checks according to that index before emitting them. This results in global value checks being emitted in the order they were seen instead of being grouped separately.
zahiraam
pushed a commit
to zahiraam/llvm-project
that referenced
this pull request
Nov 20, 2023
Recommits the changes from https://reviews.llvm.org/D148216. Explicitly named globals are now matched literally, instead of emitting a capture group for the name. This resolves llvm#70047. Metadata and annotations, on the other hand, are captured and matched against by default, since their identifiers are not stable. The reasons for revert (llvm#63746) have been fixed: The first issue, that of duplicated checkers, has already been resolved in llvm#70050. This PR resolves the second issue listed in llvm#63746, regarding the order of named and unnamed globals. This is fixed by recording the index of substrings containing global values, and sorting the checks according to that index before emitting them. This results in global value checks being emitted in the order they were seen instead of being grouped separately.
|
This broke something else: #78517 I suggest your run something like |
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.
Previously when using
-pa.k.a.--preserve-namesexisting lines for checking globals were not recognised as such, leading to the line being kept while also being emitted again, resulting in duplicated CHECK lines.This resolves #70048.