[UpdateTestChecks] Don't fail silently when conflicting CHECK lines means no checks are generated for some functions #159321

asb · 2025-09-17T11:06:37Z

There is a warning that triggers if you (for instance) run update_llc_test_checks.py on an input where all functions have conflicting check lines and so no checks are generated. However, there are no warnings emitted at all for the case where some functions have non-conflicting check lines but others don't. This is a source of frustration because running update_llc_test_checks can result in all check lines being removed for certain functions when such a conflict exists with no warning, meaning we have to be extra vigilant inspecting the diff. I've also personally wasted time tracking down the source of the dropped lines assuming that update_test_checks would emit a warning in such cases.

This change adds logic to emit warnings on a function-by-function basis for any RUN that has conflicting prefixes meaning no output is generated. This subsumes the previous warning for when all functions conflict.

CC @topperc @preames as this came up on a recent RISC-V LLVM sync call.

Note to reviewers: I am fairly happy this change addresses the issue I'm seeing for update_llc_test_checks. I believe (hope!) I've coded it defensively enough that it shouldn't generate false warnings for obscure cases, but I would of course appreciate more eyes on this.

…eans no checks are generated for some functions There is a warning that triggers if you (for instance) run `update_llc_test_checks.py` on an input where _all_ functions have conflicting check lines and so no checks are generated. However, there are no warnings emitted at all for the case where some functions have non-conflicting check lines but others don't. This is a source of frustration because running update_llc_test_checks can result in all check lines being removed when such a conflict exists with no warning, meaning we have to be extra vigilant inspecting the diff. I've also personally wasted time tracking down the source of the dropped lines assuming that update_test_checks would emit a warning in such cases. This change adds logic to emit warnings on a function-by-function basis for any RUN that has conflicting prefixes meaning no output is generated. This subsumes the previous warning for when _all_ functions conflict.

llvmbot · 2025-09-17T11:07:12Z

@llvm/pr-subscribers-testing-tools

Author: Alex Bradbury (asb)

Changes

There is a warning that triggers if you (for instance) run update_llc_test_checks.py on an input where all functions have conflicting check lines and so no checks are generated. However, there are no warnings emitted at all for the case where some functions have non-conflicting check lines but others don't. This is a source of frustration because running update_llc_test_checks can result in all check lines being removed when such a conflict exists with no warning, meaning we have to be extra vigilant inspecting the diff. I've also personally wasted time tracking down the source of the dropped lines assuming that update_test_checks would emit a warning in such cases.

This change adds logic to emit warnings on a function-by-function basis for any RUN that has conflicting prefixes meaning no output is generated. This subsumes the previous warning for when all functions conflict.

CC @topperc @preames as this came up on a recent RISC-V LLVM sync call.

Note to reviewers: I am fairly happy this change addresses the issue I'm seeing for update_llc_testing_checks. I believe (hope!) I've coded it defensively enough that it shouldn't generate false warnings, but I would of course appreciate more eyes on this.

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

5 Files Affected:

(added) llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/conflicting-prefixes.ll (+16)
(added) llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/conflicting-prefixes.ll.expected (+21)
(added) llvm/test/tools/UpdateTestChecks/update_llc_test_checks/conflicting-prefixes.test (+7)
(modified) llvm/test/tools/UpdateTestChecks/update_llc_test_checks/prefix-never-matches.test (+1-1)
(modified) llvm/utils/UpdateTestChecks/common.py (+45-20)

diff --git a/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/conflicting-prefixes.ll b/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/conflicting-prefixes.ll
new file mode 100644
index 0000000000000..fdc53951d6bb0
--- /dev/null
+++ b/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/conflicting-prefixes.ll
@@ -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
+}
diff --git a/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/conflicting-prefixes.ll.expected b/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/conflicting-prefixes.ll.expected
new file mode 100644
index 0000000000000..b3cad11e2ec1d
--- /dev/null
+++ b/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/conflicting-prefixes.ll.expected
@@ -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
+}
diff --git a/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/conflicting-prefixes.test b/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/conflicting-prefixes.test
new file mode 100644
index 0000000000000..c9a6a7268d0b3
--- /dev/null
+++ b/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/conflicting-prefixes.test
@@ -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
diff --git a/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/prefix-never-matches.test b/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/prefix-never-matches.test
index 2e75148addd84..90ae70bda64d9 100644
--- a/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/prefix-never-matches.test
+++ b/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/prefix-never-matches.test
@@ -4,5 +4,5 @@
 # RUN: %update_llc_test_checks --no-generate-body-for-unused-prefixes %t.ll 2>&1 | FileCheck %s
 # RUN: FileCheck --input-file=%t.ll %s --check-prefix=OUTPUT
 
-# CHECK: WARNING: Prefix A had conflicting output
+# CHECK: WARNING: For function 'fold_v2i64', the following RUN lines will not generate checks due to conflicting output
 # OUTPUT-NOT: A:
diff --git a/llvm/utils/UpdateTestChecks/common.py b/llvm/utils/UpdateTestChecks/common.py
index 1c795afa9e700..c1cb195852a79 100644
--- a/llvm/utils/UpdateTestChecks/common.py
+++ b/llvm/utils/UpdateTestChecks/common.py
@@ -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,52 @@ 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
+                if not run_contains_func:
+                    continue
+
+                # Check if this RUN line can print any checks for this
+                # function. It can't if all of its prefixes have conflicting
+                # (None) output.
+                can_print_for_this_run = False
+                for p in prefixes:
+                    if self._func_dict[p].get(func) is not None:
+                        can_print_for_this_run = True
+                        break
+
+                if not can_print_for_this_run:
+                    warnings_to_print[func].append((i, prefixes))
+
+        for func, warning_info in warnings_to_print.items():
+            conflict_strs = []
+            for run_index, prefixes in warning_info:
+                conflict_strs.append(
+                    "RUN #{} (prefixes: {})".format(
+                        run_index + 1, ", ".join(prefixes)
+                    )
                 )
+            warn(
+                "For function '{}', the following RUN lines will not generate checks due to conflicting output: {}".format(
+                    func, ", ".join(conflict_strs)),
+                test_file=self._path,
             )
+
         return self._func_dict
 
     def func_order(self):
@@ -1078,20 +1117,6 @@ def processed_prefixes(self, prefixes):
         """
         self._processed_prefixes.update(prefixes)
 
-    def get_failed_prefixes(self):
-        # This returns the list of those prefixes that failed to match any function,
-        # because there were conflicting bodies produced by different RUN lines, in
-        # all instances of the prefix.
-        for prefix in self._func_dict:
-            if self._func_dict[prefix] and (
-                not [
-                    fct
-                    for fct in self._func_dict[prefix]
-                    if self._func_dict[prefix][fct] is not None
-                ]
-            ):
-                yield prefix
-
 
 ##### Generator of LLVM IR CHECK lines

github-actions · 2025-09-17T11:09:00Z

✅ With the latest revision this PR passed the Python code formatter.

`-riscv-fp-imm-cost` controls the threshold at which the constant pool is used for float constants rather than generating directly (typically into a GPR followed by an `fmv`). The value used for this knob indicates the number of instructions that can be used to produce the value (otherwise we fall back to the constant pool). Upping to to 3 covers a huge number of additional constants (see <llvm#153402>), e.g. most whole numbers which can be generated through lui+shift+fmv. As in general we struggle with efficient code generation for constant pool accesses, reducing the number of constant pool accesses is beneficial. We are typically replacing a two-instruction sequence (which includes a load) with a three instruction sequence (two simple arithmetic operations plus a fmv), which. The CHECK prefixes for various tests had to be updated to avoid conflicts leading to check lines being dropped altogether (see <llvm#159321> for a change to update_llc_test_checks to aid diagnosing this).

arichardson

Thanks for fixing this, I have run into this before as well and a warning would be very useful!

llvm/utils/UpdateTestChecks/common.py

arichardson · 2025-09-17T17:25:05Z

llvm/utils/UpdateTestChecks/common.py

+                run_contains_func = True
+                for p in prefixes:
+                    if func not in self._func_dict.get(p, {}):
+                        run_contains_func = False
+                        break


Suggested change

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?

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?

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.

llvm/utils/UpdateTestChecks/common.py

…t-checks-conflict-improvement

llvm/test/tools/UpdateTestChecks/update_llc_test_checks/conflicting-prefixes.test

llvm/utils/UpdateTestChecks/common.py

…some functions aren't present for some RUN lines

…t-checks-conflict-improvement

…vement

asb · 2025-09-23T15:34:23Z

Thank you @arichardson for the quick review. I'll land this now (and use this as an opportunity to try out the "auto-merge" button!).

`-riscv-fp-imm-cost` controls the threshold at which the constant pool is used for float constants rather than generating directly (typically into a GPR followed by an `fmv`). The value used for this knob indicates the number of instructions that can be used to produce the value (otherwise we fall back to the constant pool). Upping to to 3 covers a huge number of additional constants (see <#153402>), e.g. most whole numbers which can be generated through lui+shift+fmv. As in general we struggle with efficient code generation for constant pool accesses, reducing the number of constant pool accesses is beneficial. We are typically replacing a two-instruction sequence (which includes a load) with a three instruction sequence (two simple arithmetic operations plus a fmv), which. The CHECK prefixes for various tests had to be updated to avoid conflicts leading to check lines being dropped altogether (see <#159321> for a change to update_llc_test_checks to aid diagnosing this).

…59352) `-riscv-fp-imm-cost` controls the threshold at which the constant pool is used for float constants rather than generating directly (typically into a GPR followed by an `fmv`). The value used for this knob indicates the number of instructions that can be used to produce the value (otherwise we fall back to the constant pool). Upping to to 3 covers a huge number of additional constants (see <llvm/llvm-project#153402>), e.g. most whole numbers which can be generated through lui+shift+fmv. As in general we struggle with efficient code generation for constant pool accesses, reducing the number of constant pool accesses is beneficial. We are typically replacing a two-instruction sequence (which includes a load) with a three instruction sequence (two simple arithmetic operations plus a fmv), which. The CHECK prefixes for various tests had to be updated to avoid conflicts leading to check lines being dropped altogether (see <llvm/llvm-project#159321> for a change to update_llc_test_checks to aid diagnosing this).

Since llvm#159321 we now get actual warnings when we're missing coverage

Since #159321 we now get actual warnings when we're missing coverage

Since llvm#159321 we now get actual warnings when we're missing coverage

These are all cases where check lines were being silently dropped prior to llvm#159321 which added proper warnings. I did `find llvm/test/CodeGen/RISCV -name "*.ll" -exec ./llvm/utils/update_llc_test_checks.py --llc-bin=./remote-llc -u {} \;` and went through all cases that emitted the new warning.

Since #159321 we now get actual warnings when we're missing coverage

…#160689) These are all cases where check lines were being silently dropped prior to #159321 which added proper warnings. I did `find llvm/test/CodeGen/RISCV -name "*.ll" -exec ./llvm/utils/update_llc_test_checks.py --llc-bin=./remote-llc -u {} \;` and went through all cases that emitted the new warning. `idiv_large.ll` is a case that seems to not be generated by update_llc_test_checks but still has the comment indicating it was (presumably it was hand-edited after generation).

jayfoad · 2025-10-02T14:03:25Z

Thanks for fixing this! It was super annoying.

Since llvm#159321 we now get actual warnings when we're missing coverage

…llvm#160689) These are all cases where check lines were being silently dropped prior to llvm#159321 which added proper warnings. I did `find llvm/test/CodeGen/RISCV -name "*.ll" -exec ./llvm/utils/update_llc_test_checks.py --llc-bin=./remote-llc -u {} \;` and went through all cases that emitted the new warning. `idiv_large.ll` is a case that seems to not be generated by update_llc_test_checks but still has the comment indicating it was (presumably it was hand-edited after generation).

asb requested review from preames, nhaehnle and topperc September 17, 2025 11:06

llvmbot added the testing-tools label Sep 17, 2025

Apply reformatting suggested by darker

3d8c2a8

RKSimon mentioned this pull request Sep 17, 2025

update_llc_test_checks.py doesn't report when a common prefix has been lost #48533

Closed

asb mentioned this pull request Sep 17, 2025

[RISCV] Set riscv-fpimm-cost threshold to 3 by default #159352

Merged

arichardson reviewed Sep 17, 2025

View reviewed changes

asb added 2 commits September 22, 2025 11:36

Address review comments

be93e6e

Merge remote-tracking branch 'origin/main' into 2025q3-update-llc-tes…

66ba467

…t-checks-conflict-improvement

arichardson approved these changes Sep 22, 2025

View reviewed changes

llvm/test/tools/UpdateTestChecks/update_llc_test_checks/conflicting-prefixes.test Outdated Show resolved Hide resolved

arichardson reviewed Sep 22, 2025

View reviewed changes

llvm/utils/UpdateTestChecks/common.py Outdated Show resolved Hide resolved

asb added 6 commits September 23, 2025 10:24

Check full warning in conflicting-prefixes.test

c3cc1ba

Avoid double not

4ca0f22

Add test case that checks we don't print false positive warning when …

e8ffdc9

…some functions aren't present for some RUN lines

Attempt to fix darker code formatting issues

7e47a56

Merge remote-tracking branch 'origin/main' into 2025q3-update-llc-tes…

327c1ba

…t-checks-conflict-improvement

Merge branch 'main' into 2025q3-update-llc-test-checks-conflict-impro…

bdbcc77

…vement

asb enabled auto-merge (squash) September 23, 2025 15:32

asb merged commit 9d48df7 into llvm:main Sep 23, 2025
9 checks passed

RKSimon added a commit to RKSimon/llvm-project that referenced this pull request Sep 25, 2025

[X86] Add missing prefixes to trunc-sat tests

bf6190e

Since llvm#159321 we now get actual warnings when we're missing coverage

RKSimon mentioned this pull request Sep 25, 2025

[X86] Add missing prefixes to trunc-sat tests #160662

Merged

RKSimon added a commit that referenced this pull request Sep 25, 2025

[X86] Add missing prefixes to trunc-sat tests (#160662)

28a8dfb

Since #159321 we now get actual warnings when we're missing coverage

RKSimon added a commit to RKSimon/llvm-project that referenced this pull request Sep 25, 2025

[X86] masked_store_trunc.ll - add missing prefixes

b5d6fdf

Since llvm#159321 we now get actual warnings when we're missing coverage

RKSimon mentioned this pull request Sep 25, 2025

[X86] masked_store_trunc.ll - add missing prefixes #160685

Merged

asb mentioned this pull request Sep 25, 2025

[RISCV][test] Add additional check-prefixes to fix conflicts in tests #160689

Merged

RKSimon added a commit that referenced this pull request Sep 25, 2025

[X86] masked_store_trunc.ll - add missing prefixes (#160685)

48a0bb7

Since #159321 we now get actual warnings when we're missing coverage

mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025

[X86] Add missing prefixes to trunc-sat tests (llvm#160662)

608e733

Since llvm#159321 we now get actual warnings when we're missing coverage

mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025

[X86] masked_store_trunc.ll - add missing prefixes (llvm#160685)

f4cf7ec

Since llvm#159321 we now get actual warnings when we're missing coverage

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[UpdateTestChecks] Don't fail silently when conflicting CHECK lines means no checks are generated for some functions #159321

[UpdateTestChecks] Don't fail silently when conflicting CHECK lines means no checks are generated for some functions #159321

Uh oh!

asb commented Sep 17, 2025 •

edited

Loading

Uh oh!

llvmbot commented Sep 17, 2025

Uh oh!

github-actions bot commented Sep 17, 2025 •

edited

Loading

Uh oh!

arichardson left a comment

Uh oh!

Uh oh!

Uh oh!

arichardson Sep 17, 2025

Uh oh!

arichardson Sep 17, 2025

Uh oh!

asb Sep 23, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asb commented Sep 23, 2025

Uh oh!

Uh oh!

jayfoad commented Oct 2, 2025

Uh oh!

Uh oh!

[UpdateTestChecks] Don't fail silently when conflicting CHECK lines means no checks are generated for some functions #159321

[UpdateTestChecks] Don't fail silently when conflicting CHECK lines means no checks are generated for some functions #159321

Uh oh!

Conversation

asb commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Sep 17, 2025

Uh oh!

github-actions bot commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arichardson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

arichardson Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

arichardson Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

asb Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asb commented Sep 23, 2025

Uh oh!

Uh oh!

jayfoad commented Oct 2, 2025

Uh oh!

Uh oh!

asb commented Sep 17, 2025 •

edited

Loading

github-actions bot commented Sep 17, 2025 •

edited

Loading