-
Notifications
You must be signed in to change notification settings - Fork 15k
[UpdateTestChecks] Don't fail silently when conflicting CHECK lines means no checks are generated for some functions #159321
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
Changes from 2 commits
a3bd517
3d8c2a8
be93e6e
66ba467
c3cc1ba
4ca0f22
e8ffdc9
7e47a56
327c1ba
bdbcc77
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| ; RUN: sed 's/RETVAL/1/g' %s | llc -mtriple=riscv32 \ | ||
| ; RUN: | FileCheck -check-prefixes=CHECK,CHECKA %s | ||
| ; RUN: sed 's/RETVAL/2/g' %s | llc -mtriple=riscv32 \ | ||
| ; RUN: | FileCheck -check-prefixes=CHECK,CHECKA %s | ||
| ; RUN: sed 's/RETVAL/3/g' %s | llc -mtriple=riscv32 \ | ||
| ; RUN: | FileCheck -check-prefixes=CHECK,CHECKB %s | ||
| ; RUN: sed 's/RETVAL/4/g' %s | llc -mtriple=riscv32 \ | ||
| ; RUN: | FileCheck -check-prefixes=CHECK,CHECKB %s | ||
|
|
||
| define i32 @foo() { | ||
| ret i32 RETVAL | ||
| } | ||
|
|
||
| define i32 @bar() { | ||
| ret i32 100 | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --no-generate-body-for-unused-prefixes | ||
| ; RUN: sed 's/RETVAL/1/g' %s | llc -mtriple=riscv32 \ | ||
| ; RUN: | FileCheck -check-prefixes=CHECK,CHECKA %s | ||
| ; RUN: sed 's/RETVAL/2/g' %s | llc -mtriple=riscv32 \ | ||
| ; RUN: | FileCheck -check-prefixes=CHECK,CHECKA %s | ||
| ; RUN: sed 's/RETVAL/3/g' %s | llc -mtriple=riscv32 \ | ||
| ; RUN: | FileCheck -check-prefixes=CHECK,CHECKB %s | ||
| ; RUN: sed 's/RETVAL/4/g' %s | llc -mtriple=riscv32 \ | ||
| ; RUN: | FileCheck -check-prefixes=CHECK,CHECKB %s | ||
|
|
||
| define i32 @foo() { | ||
| ret i32 RETVAL | ||
| } | ||
|
|
||
| define i32 @bar() { | ||
| ; CHECK-LABEL: bar: | ||
| ; CHECK: # %bb.0: | ||
| ; CHECK-NEXT: li a0, 100 | ||
| ; CHECK-NEXT: ret | ||
| ret i32 100 | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # REQUIRES: riscv-registered-target | ||
|
|
||
| # RUN: cp -f %S/Inputs/conflicting-prefixes.ll %t.ll | ||
| # RUN: %update_llc_test_checks --no-generate-body-for-unused-prefixes %t.ll 2>&1 | FileCheck %s | ||
| # RUN: diff -u %S/Inputs/conflicting-prefixes.ll.expected %t.ll | ||
|
|
||
| # CHECK: WARNING: For function 'foo', the following RUN lines will not generate checks due to conflicting output | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -882,6 +882,7 @@ def __str__(self): | |||||||||||||
|
|
||||||||||||||
| class FunctionTestBuilder: | ||||||||||||||
| def __init__(self, run_list, flags, scrubber_args, path, ginfo): | ||||||||||||||
| self._run_list = run_list | ||||||||||||||
| self._verbose = flags.verbose | ||||||||||||||
| self._record_args = flags.function_signature | ||||||||||||||
| self._check_attributes = flags.check_attributes | ||||||||||||||
|
|
@@ -918,14 +919,51 @@ def __init__(self, run_list, flags, scrubber_args, path, ginfo): | |||||||||||||
| self._global_var_dict.update({prefix: dict()}) | ||||||||||||||
|
|
||||||||||||||
| def finish_and_get_func_dict(self): | ||||||||||||||
| for prefix in self.get_failed_prefixes(): | ||||||||||||||
| warn( | ||||||||||||||
| "Prefix %s had conflicting output from different RUN lines for all functions in test %s" | ||||||||||||||
| % ( | ||||||||||||||
| prefix, | ||||||||||||||
| self._path, | ||||||||||||||
| all_funcs = set() | ||||||||||||||
| for prefix in self._func_dict: | ||||||||||||||
| all_funcs.update(self._func_dict[prefix].keys()) | ||||||||||||||
|
|
||||||||||||||
| warnings_to_print = collections.defaultdict(list) | ||||||||||||||
| for func in sorted(list(all_funcs)): | ||||||||||||||
| for i, run_info in enumerate(self._run_list): | ||||||||||||||
| prefixes = run_info[0] | ||||||||||||||
| if not prefixes: | ||||||||||||||
| continue | ||||||||||||||
|
|
||||||||||||||
| # Check if this RUN line produces this function at all. | ||||||||||||||
| run_contains_func = True | ||||||||||||||
| for p in prefixes: | ||||||||||||||
| if func not in self._func_dict.get(p, {}): | ||||||||||||||
| run_contains_func = False | ||||||||||||||
| break | ||||||||||||||
|
||||||||||||||
| run_contains_func = True | |
| for p in prefixes: | |
| if func not in self._func_dict.get(p, {}): | |
| run_contains_func = False | |
| break | |
| run_contains_func = all(func in self._func_dict.get(p, {}) for p in prefixes) |
I think this should be equivalent any maybe easier to read?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I read this correctly, you are checking that all prefixes have a func_dict entry for this function?
I have never looked into this part of the update_test_checks script, so I feel this needs a comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've extended the comment to hopefully clarify, and also added a test case that captures this behaviour. When I thought about potential corner cases that could produce a false positive, this is the one that came to mind. Without this logic we do seem to produce a confusing warning for the --included-generated-funcs test case I committed. Possibly there are still cases with non-matching sets of functions and prefix clashes we would want to error, but it's not clear to me what behaviour is right. Either way, I think this patch is a strict improvement even if it's possible there are corner cases like this where maybe you want even more error checking.
Uh oh!
There was an error while loading. Please reload this page.