[utils] Fix UpdateTestChecks case where 2 runs differ for last label

[mtrofin]

Summary

Two RUN lines produce outputs that, each, have some common parts and
some different parts. The common parts are checked under label A. The
differing parts are associated to a function and checked under labels B
and C, respectivelly.
When build_function_body_dictionary is called for the first RUN line, it
will attribute the function body to labels A and C. When the second RUN
is passed to build_function_body_dictionary, it sees that the function
body under A is different from what it has. If in this second RUN line,
A were at the end of the prefixes list, A's body is still kept
associated with the first run's function.

When we output the function body (i.e. add_checks), we stop after
emitting for the first prefix matching that function. So we end up with
the wrong function body (first RUN's A-association).

There is no reason to special-case the last label in the prefixes list,
and the fix is to always clear a label association if we find a RUN line
where the body is different.

[pengfei]

Add initial author to have a review.

llvm/utils/UpdateTestChecks/common.py

296

I didn't dig into it too much, but the code looks to me that if all prefixes for the func don't meet the condition (already iterated to the last one), it reports warnings to user.
It seems you just removed the warnings instead of fixing the problem. So users hardly be aware of there's real conflict on their prefixes lists on RUN command line.

[mtrofin]

llvm/utils/UpdateTestChecks/common.py

296

I'm not sure what the original intent was there, but I'm not sure it serves a real reason anymore. See for example the "common-label-different-bodies-2.ll" test case, which would fail today (i.e. it'd produce the wrong outcome, and warn) - can't see why that would be desirable. Maybe I'm missing something?

[pengfei]

llvm/utils/UpdateTestChecks/common.py

296

I did an experiment: if you change both prefixes of the RUNs in your tests to the same one, you will find it doesn't generate checks for them and doesn't report any warning. I guess that's should be the original intent here.

[mtrofin]

llvm/utils/UpdateTestChecks/common.py

296

Ah, thanks! Like so (picking on the -2 file):

; RUN: llc < %s -mtriple=i686-unknown-linux-gnu -mattr=+sse2 | FileCheck %s --check-prefixes=A
; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefixes=A

Got it. Indeed, the proposed behavior doesn't warn and doesn't generate an output for A; and the old behavior was generating incorrect asserts and outputing a warning.

This seems to end up as an unused prefixes case - (or am I holding a hammer and everything is a nail?). I suppose we can handle it in the tool after all the RUN lines are processed, but there are a few users of build_function_body_dictionary, so it'd be best to wrap the func_dict population; or, we can let it silently fail, and then FileCheck will complain quickly thereafter about duplicate prefixes.

Happy to do the wrapper in the tools, especially if this is something that happens frequently?

[mtrofin]

added check for when a prefix has conflicts for all functions

[pengfei]

What's the difference with the existing code? It looks to me that you just brought the warning out of loop, right?

llvm/utils/update_analyze_test_checks.py

128 ↗

(On Diff #311359)

redundant space.

129 ↗

(On Diff #311359)

Can we move these warn to common.py?

llvm/utils/update_cc_test_checks.py

269 ↗

(On Diff #311359)

redundant space.

llvm/utils/update_test_checks.py

120 ↗

(On Diff #311359)

redundant space.

[mtrofin]

In D93078#2451747, @pengfei wrote:

What's the difference with the existing code? It looks to me that you just brought the warning out of loop, right?

Oh true! we can just do the check in build_function_body_dictionary_for_triple at the end.

I'll leave the additional tests, though, more tests never hurt.

[mtrofin]

fix

[mtrofin]

cleanup

[pengfei]

LGTM. Thanks.
update_test_prefix.py assumes the conflicting output. You may need to change the expection of it as well.

[mtrofin]

In D93078#2453891, @pengfei wrote:

LGTM. Thanks.
update_test_prefix.py assumes the conflicting output. You may need to change the expection of it as well.

oh yes - made it check the new warning. Thanks!

[mtrofin]

update_test_prefix.py change

[mtrofin]

llvm/utils/update_analyze_test_checks.py

129 ↗

(On Diff #311359)

Come to think of it, maybe moving it to common.py was not quite ideal:

• once a warning is issued in the middle of the list of RUN lines, it'll just be re-issued next time around; we could warn and exit, but that'd be annoying if there's another failure later in the RUN list.
• if we do it at the end, we can additionally distinguish the case where a prefix has an empty dict of funcs associated with it -> and the warning would be "there are unused prefixes - please remove %s'. This is more discoverable than llvm-lit -a, and also than relying on the user knowing to run uplate_test_prefix.

I think that the benefits (discoverability, better, more concise warnings) make the extra 2-3 lines worth it; and we already have tests for these other tools - wdyt?

[pengfei]

llvm/utils/update_analyze_test_checks.py

129 ↗

(On Diff #311359)

Make sence for me. Thanks for continuing to improve it.

[jdoerfert]

I think this caused a lot of problems for the Attributor tests. I get the "conflict output" warning all the time now :(

These two UpdateTestChecks tests also generate the warning. I would think that they haven't before.

LLVM :: tools/UpdateTestChecks/update_test_checks/check_attrs.test
LLVM :: tools/UpdateTestChecks/update_test_checks/prefix-never-matches.test

I suspect the --check-attributes flag effect are not taken into account somewhere but I might be wrong.

[mtrofin]

In D93078#2499666, @jdoerfert wrote:

I think this caused a lot of problems for the Attributor tests. I get the "conflict output" warning all the time now :(

These two UpdateTestChecks tests also generate the warning. I would think that they haven't before.

LLVM :: tools/UpdateTestChecks/update_test_checks/check_attrs.test
LLVM :: tools/UpdateTestChecks/update_test_checks/prefix-never-matches.test

I suspect the --check-attributes flag effect are not taken into account somewhere but I might be wrong.

The warnings are correct, but their purpose is to inform. For example, take check_attrs. The third and fourth opt run produce different function attributes from those produced by the first two, so the function is different insofar as update_test_prefix is concerned. The warnings are there to indicate that the listed prefixes end up not being used. But, that's 'by design' for the attributor tests, IIRC.

If the warnings are noise for you, we could add a flag to quiet them down, which you'd flip when regenerating attributor tests?

[jdoerfert]

In D93078#2499982, @mtrofin wrote:

In D93078#2499666, @jdoerfert wrote:

I think this caused a lot of problems for the Attributor tests. I get the "conflict output" warning all the time now :(

These two UpdateTestChecks tests also generate the warning. I would think that they haven't before.

LLVM :: tools/UpdateTestChecks/update_test_checks/check_attrs.test
LLVM :: tools/UpdateTestChecks/update_test_checks/prefix-never-matches.test

I suspect the --check-attributes flag effect are not taken into account somewhere but I might be wrong.

The warnings are correct, but their purpose is to inform. For example, take check_attrs. The third and fourth opt run produce different function attributes from those produced by the first two, so the function is different insofar as update_test_prefix is concerned. The warnings are there to indicate that the listed prefixes end up not being used. But, that's 'by design' for the attributor tests, IIRC.

If the warnings are noise for you, we could add a flag to quiet them down, which you'd flip when regenerating attributor tests?

TBH, before, the warnings meant there is a problem that needs fixing. Now they mean, there might be one or not. So, depending on your setup it's just noise while it was pure signal before.
I'm not sure how this is more helpful. What is the use case where this way of warning helps?

[mtrofin]

In D93078#2499995, @jdoerfert wrote:

In D93078#2499982, @mtrofin wrote:

In D93078#2499666, @jdoerfert wrote:

I think this caused a lot of problems for the Attributor tests. I get the "conflict output" warning all the time now :(

These two UpdateTestChecks tests also generate the warning. I would think that they haven't before.

LLVM :: tools/UpdateTestChecks/update_test_checks/check_attrs.test
LLVM :: tools/UpdateTestChecks/update_test_checks/prefix-never-matches.test

I suspect the --check-attributes flag effect are not taken into account somewhere but I might be wrong.

The warnings are correct, but their purpose is to inform. For example, take check_attrs. The third and fourth opt run produce different function attributes from those produced by the first two, so the function is different insofar as update_test_prefix is concerned. The warnings are there to indicate that the listed prefixes end up not being used. But, that's 'by design' for the attributor tests, IIRC.

If the warnings are noise for you, we could add a flag to quiet them down, which you'd flip when regenerating attributor tests?

TBH, before, the warnings meant there is a problem that needs fixing. Now they mean, there might be one or not. So, depending on your setup it's just noise while it was pure signal before.
I'm not sure how this is more helpful. What is the use case where this way of warning helps?

For tests other than attributor, that explicitly set FileCheck --allow-unused-prefixes=true, these warnings mean that there will be unused prefixes (those listed)

[jdoerfert]

In D93078#2499996, @mtrofin wrote:

In D93078#2499995, @jdoerfert wrote:

In D93078#2499982, @mtrofin wrote:

In D93078#2499666, @jdoerfert wrote:

I think this caused a lot of problems for the Attributor tests. I get the "conflict output" warning all the time now :(

These two UpdateTestChecks tests also generate the warning. I would think that they haven't before.

LLVM :: tools/UpdateTestChecks/update_test_checks/check_attrs.test
LLVM :: tools/UpdateTestChecks/update_test_checks/prefix-never-matches.test

I suspect the --check-attributes flag effect are not taken into account somewhere but I might be wrong.

The warnings are correct, but their purpose is to inform. For example, take check_attrs. The third and fourth opt run produce different function attributes from those produced by the first two, so the function is different insofar as update_test_prefix is concerned. The warnings are there to indicate that the listed prefixes end up not being used. But, that's 'by design' for the attributor tests, IIRC.

If the warnings are noise for you, we could add a flag to quiet them down, which you'd flip when regenerating attributor tests?

TBH, before, the warnings meant there is a problem that needs fixing. Now they mean, there might be one or not. So, depending on your setup it's just noise while it was pure signal before.
I'm not sure how this is more helpful. What is the use case where this way of warning helps?

For tests other than attributor, that explicitly set FileCheck --allow-unused-prefixes=true, these warnings mean that there will be unused prefixes (those listed)

Should not we check for that flag in the RUN line then and only warn for unused prefixes when it is set. If there is no prefix we should obviously always warn.

[mtrofin]

In D93078#2500032, @jdoerfert wrote:

In D93078#2499996, @mtrofin wrote:

In D93078#2499995, @jdoerfert wrote:

In D93078#2499982, @mtrofin wrote:

In D93078#2499666, @jdoerfert wrote:

I think this caused a lot of problems for the Attributor tests. I get the "conflict output" warning all the time now :(

These two UpdateTestChecks tests also generate the warning. I would think that they haven't before.

LLVM :: tools/UpdateTestChecks/update_test_checks/check_attrs.test
LLVM :: tools/UpdateTestChecks/update_test_checks/prefix-never-matches.test

I suspect the --check-attributes flag effect are not taken into account somewhere but I might be wrong.

The warnings are correct, but their purpose is to inform. For example, take check_attrs. The third and fourth opt run produce different function attributes from those produced by the first two, so the function is different insofar as update_test_prefix is concerned. The warnings are there to indicate that the listed prefixes end up not being used. But, that's 'by design' for the attributor tests, IIRC.

If the warnings are noise for you, we could add a flag to quiet them down, which you'd flip when regenerating attributor tests?

TBH, before, the warnings meant there is a problem that needs fixing. Now they mean, there might be one or not. So, depending on your setup it's just noise while it was pure signal before.
I'm not sure how this is more helpful. What is the use case where this way of warning helps?

For tests other than attributor, that explicitly set FileCheck --allow-unused-prefixes=true, these warnings mean that there will be unused prefixes (those listed)

Should not we check for that flag in the RUN line then and only warn for unused prefixes when it is set. If there is no prefix we should obviously always warn.

That's a good idea. Probably we'd need to also make sure that the unused prefixes are all on RUN lines with --allow-unused-prefixes=true.

I'm also not sure how lit.local.cfg interacts with the test prefix updater: currently, the only cases where we bulk-want to allow unused prefixes is the Attributor tests. If you were going to add the flag explicitly, that'd also work. Or just the option to the update_test_prefix that says "ok with duplicates, don't warn"

[jdoerfert]

In D93078#2500040, @mtrofin wrote:

In D93078#2500032, @jdoerfert wrote:

In D93078#2499996, @mtrofin wrote:

In D93078#2499995, @jdoerfert wrote:

I'm not sure how this is more helpful. What is the use case where this way of warning helps?

For tests other than attributor, that explicitly set FileCheck --allow-unused-prefixes=true, these warnings mean that there will be unused prefixes (those listed)

Should not we check for that flag in the RUN line then and only warn for unused prefixes when it is set. If there is no prefix we should obviously always warn.

That's a good idea. Probably we'd need to also make sure that the unused prefixes are all on RUN lines with --allow-unused-prefixes=true.

I'm also not sure how lit.local.cfg interacts with the test prefix updater: currently, the only cases where we bulk-want to allow unused prefixes is the Attributor tests. If you were going to add the flag explicitly, that'd also work. Or just the option to the update_test_prefix that says "ok with duplicates, don't warn"

I can add the option explicitly (D94744). We should look for the filecheck one if possible, two options means double the hassle. That said, why are we warning in both FileCheck and update_test_check, it seems to be unnecessary to do the latter.

[mtrofin]

In D93078#2500114, @jdoerfert wrote:

In D93078#2500040, @mtrofin wrote:

In D93078#2500032, @jdoerfert wrote:

In D93078#2499996, @mtrofin wrote:

In D93078#2499995, @jdoerfert wrote:

I'm not sure how this is more helpful. What is the use case where this way of warning helps?

For tests other than attributor, that explicitly set FileCheck --allow-unused-prefixes=true, these warnings mean that there will be unused prefixes (those listed)

Should not we check for that flag in the RUN line then and only warn for unused prefixes when it is set. If there is no prefix we should obviously always warn.

That's a good idea. Probably we'd need to also make sure that the unused prefixes are all on RUN lines with --allow-unused-prefixes=true.

I'm also not sure how lit.local.cfg interacts with the test prefix updater: currently, the only cases where we bulk-want to allow unused prefixes is the Attributor tests. If you were going to add the flag explicitly, that'd also work. Or just the option to the update_test_prefix that says "ok with duplicates, don't warn"

I can add the option explicitly (D94744). We should look for the filecheck one if possible, two options means double the hassle. That said, why are we warning in both FileCheck and update_test_check, it seems to be unnecessary to do the latter.

update_test_prefix.py's role is temporary: once we flip FileCheck to disallow unused prefixes by default, we don't need to keep it around. At that point, it becomes important for the update_<xyz>_test_check scripts to warn.

[jdoerfert]

In D93078#2500122, @mtrofin wrote:

In D93078#2500114, @jdoerfert wrote:

In D93078#2500040, @mtrofin wrote:

In D93078#2500032, @jdoerfert wrote:

In D93078#2499996, @mtrofin wrote:

In D93078#2499995, @jdoerfert wrote:

I'm not sure how this is more helpful. What is the use case where this way of warning helps?

For tests other than attributor, that explicitly set FileCheck --allow-unused-prefixes=true, these warnings mean that there will be unused prefixes (those listed)

Should not we check for that flag in the RUN line then and only warn for unused prefixes when it is set. If there is no prefix we should obviously always warn.

That's a good idea. Probably we'd need to also make sure that the unused prefixes are all on RUN lines with --allow-unused-prefixes=true.

I'm also not sure how lit.local.cfg interacts with the test prefix updater: currently, the only cases where we bulk-want to allow unused prefixes is the Attributor tests. If you were going to add the flag explicitly, that'd also work. Or just the option to the update_test_prefix that says "ok with duplicates, don't warn"

I can add the option explicitly (D94744). We should look for the filecheck one if possible, two options means double the hassle. That said, why are we warning in both FileCheck and update_test_check, it seems to be unnecessary to do the latter.

update_test_prefix.py's role is temporary: once we flip FileCheck to disallow unused prefixes by default, we don't need to keep it around. At that point, it becomes important for the update_<xyz>_test_check scripts to warn.

I don't follow. I would assume it's the opposite. If FileCheck doesn't allow unused prefixes why warn in the update script as well. Anyway, there should be a way to opt-out.

[mtrofin]

In D93078#2500124, @jdoerfert wrote:

In D93078#2500122, @mtrofin wrote:

In D93078#2500114, @jdoerfert wrote:

In D93078#2500040, @mtrofin wrote:

In D93078#2500032, @jdoerfert wrote:

In D93078#2499996, @mtrofin wrote:

In D93078#2499995, @jdoerfert wrote:

I'm not sure how this is more helpful. What is the use case where this way of warning helps?

For tests other than attributor, that explicitly set FileCheck --allow-unused-prefixes=true, these warnings mean that there will be unused prefixes (those listed)

Should not we check for that flag in the RUN line then and only warn for unused prefixes when it is set. If there is no prefix we should obviously always warn.

That's a good idea. Probably we'd need to also make sure that the unused prefixes are all on RUN lines with --allow-unused-prefixes=true.

I'm also not sure how lit.local.cfg interacts with the test prefix updater: currently, the only cases where we bulk-want to allow unused prefixes is the Attributor tests. If you were going to add the flag explicitly, that'd also work. Or just the option to the update_test_prefix that says "ok with duplicates, don't warn"

I can add the option explicitly (D94744). We should look for the filecheck one if possible, two options means double the hassle. That said, why are we warning in both FileCheck and update_test_check, it seems to be unnecessary to do the latter.

update_test_prefix.py's role is temporary: once we flip FileCheck to disallow unused prefixes by default, we don't need to keep it around. At that point, it becomes important for the update_<xyz>_test_check scripts to warn.

I don't follow. I would assume it's the opposite. If FileCheck doesn't allow unused prefixes why warn in the update script as well. Anyway, there should be a way to opt-out.

In D93078#2500124, @jdoerfert wrote:

In D93078#2500122, @mtrofin wrote:

In D93078#2500114, @jdoerfert wrote:

In D93078#2500040, @mtrofin wrote:

In D93078#2500032, @jdoerfert wrote:

In D93078#2499996, @mtrofin wrote:

In D93078#2499995, @jdoerfert wrote:

I'm not sure how this is more helpful. What is the use case where this way of warning helps?

For tests other than attributor, that explicitly set FileCheck --allow-unused-prefixes=true, these warnings mean that there will be unused prefixes (those listed)

Should not we check for that flag in the RUN line then and only warn for unused prefixes when it is set. If there is no prefix we should obviously always warn.

That's a good idea. Probably we'd need to also make sure that the unused prefixes are all on RUN lines with --allow-unused-prefixes=true.

I'm also not sure how lit.local.cfg interacts with the test prefix updater: currently, the only cases where we bulk-want to allow unused prefixes is the Attributor tests. If you were going to add the flag explicitly, that'd also work. Or just the option to the update_test_prefix that says "ok with duplicates, don't warn"

I can add the option explicitly (D94744). We should look for the filecheck one if possible, two options means double the hassle. That said, why are we warning in both FileCheck and update_test_check, it seems to be unnecessary to do the latter.

update_test_prefix.py's role is temporary: once we flip FileCheck to disallow unused prefixes by default, we don't need to keep it around. At that point, it becomes important for the update_<xyz>_test_check scripts to warn.

I don't follow. I would assume it's the opposite. If FileCheck doesn't allow unused prefixes why warn in the update script as well. Anyway, there should be a way to opt-out.

There's an earlier comment in this patch about that, anchored to line 296 of common.py. IIUC, a developer: 1) updates their test by adding new RUN lines, maybe adding prefixes to existing RUN lines, 2) runs the appropriate update_xyz_checks.py. Then, indeed, they could run the test and see FileCheck's warnings, but they might be confused. Instead, we can pre-warn. Eventually, we could imagine giving a bit more information in the warning as to which RUN line that was, for example.

I'll look at adding --allow-unused-prefixes=true awareness shortly.

clang/test/utils/update_cc_test_checks/Inputs/prefix-never-matches.cpp

+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -O0 -o - %s | FileCheck %s -check-prefix=A
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -O3 -o - %s | FileCheck %s -check-prefix=A
+
+int foo(int i ) {
+    return 1;
+}
 No newline at end of file

clang/test/utils/update_cc_test_checks/prefix-never-matches.test

+# RUN: cp -f %S/Inputs/prefix-never-matches.cpp %t.cpp
+# RUN: %update_cc_test_checks %t.cpp 2>&1 | FileCheck %s
+# RUN: FileCheck --input-file=%t.cpp %s --check-prefix=OUTPUT
+
+# CHECK: WARNING: Prefix A had conflicting output
+# OUTPUT-NOT: A:
 No newline at end of file

llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/common-label-different-bodies-1.ll

+; RUN: llc < %s -mtriple=i686-unknown-linux-gnu -mattr=+sse2 | FileCheck %s --check-prefixes=A,B
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --allow-unused-prefixes=true --check-prefixes=C,A,UNUSED
+
+declare <2 x i64> @llvm.bswap.v2i64(<2 x i64>)
+; A: declare <2 x i64> @llvm.bswap.v2i64(<2 x i64>)
+
+define <2 x i64> @fold_v2i64() {
+entry:
+  %r = call <2 x i64> @llvm.bswap.v2i64(<2 x i64> <i64 255, i64 -1>)
+  ret <2 x i64> %r
+}

llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/common-label-different-bodies-2.ll

+; RUN: llc < %s -mtriple=i686-unknown-linux-gnu -mattr=+sse2 | FileCheck %s --check-prefixes=A,B
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefixes=C,A
+
+declare <2 x i64> @llvm.bswap.v2i64(<2 x i64>)
+; A: declare <2 x i64> @llvm.bswap.v2i64(<2 x i64>)
+
+define <2 x i64> @fold_v2i64() {
+entry:
+  %r = call <2 x i64> @llvm.bswap.v2i64(<2 x i64> <i64 255, i64 -1>)
+  ret <2 x i64> %r
+}

llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/common-label-different-bodies-3.ll

+; RUN: llc < %s -mtriple=i686-unknown-linux-gnu -mattr=+sse2 | FileCheck %s --check-prefixes=A,B
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefixes=A,C
+
+declare <2 x i64> @llvm.bswap.v2i64(<2 x i64>)
+; A: declare <2 x i64> @llvm.bswap.v2i64(<2 x i64>)
+
+define <2 x i64> @fold_v2i64() {
+entry:
+  %r = call <2 x i64> @llvm.bswap.v2i64(<2 x i64> <i64 255, i64 -1>)
+  ret <2 x i64> %r
+}

llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/prefix-never-matches.ll

+; RUN: llc < %s -mtriple=i686-unknown-linux-gnu -mattr=+sse2 | FileCheck %s --check-prefixes=A
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefixes=A
+
+declare <2 x i64> @llvm.bswap.v2i64(<2 x i64>)
+
+define <2 x i64> @fold_v2i64() {
+entry:
+  %r = call <2 x i64> @llvm.bswap.v2i64(<2 x i64> <i64 255, i64 -1>)
+  ret <2 x i64> %r
+}

llvm/test/tools/UpdateTestChecks/update_llc_test_checks/common-label-different-bodies.test

+# REQUIRES: x86-registered-target
+
+# RUN: cp -f %S/Inputs/common-label-different-bodies-1.ll %t-1.ll
+# RUN: cp -f %S/Inputs/common-label-different-bodies-2.ll %t-2.ll
+# RUN: cp -f %S/Inputs/common-label-different-bodies-3.ll %t-3.ll
+# RUN: %update_llc_test_checks %t-1.ll
+# RUN: %update_llc_test_checks %t-2.ll
+# RUN: %update_llc_test_checks %t-3.ll
+# RUN: FileCheck --input-file=%t-1.ll %s
+# RUN: FileCheck --input-file=%t-2.ll %s
+# RUN: FileCheck --input-file=%t-3.ll %s
+
+# CHECK: B-LABEL: fold_v2i64
+# CHECK-NOT: A-LABEL: fold_v2i64
 No newline at end of file

llvm/test/tools/UpdateTestChecks/update_llc_test_checks/prefix-never-matches.test

+# REQUIRES: x86-registered-target
+
+# RUN: cp -f %S/Inputs/prefix-never-matches.ll %t.ll
+# RUN: %update_llc_test_checks %t.ll 2>&1 | FileCheck %s
+# RUN: FileCheck --input-file=%t.ll %s --check-prefix=OUTPUT
+
+# CHECK: WARNING: Prefix A had conflicting output
+# OUTPUT-NOT: A:
 No newline at end of file

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/prefix-never-matches.ll

+; RUN: opt -O0 -S < %s  | FileCheck %s -check-prefix=A
+; RUN: opt -O3 -S < %s  | FileCheck %s -check-prefix=A
+
+define i32 @foo(i32 %i) {
+    %r = add i32 1, 1
+    ret i32 %r
+}
 No newline at end of file

llvm/test/tools/UpdateTestChecks/update_test_checks/prefix-never-matches.test

+# RUN: cp -f %S/Inputs/prefix-never-matches.ll %t.ll
+# RUN: %update_test_checks %t.ll 2>&1 | FileCheck %s
+# RUN: FileCheck --input-file=%t.ll %s --check-prefix=OUTPUT
+
+# CHECK: WARNING: Prefix A had conflicting output
+# OUTPUT-NOT: A:
 No newline at end of file

llvm/utils/UpdateTestChecks/common.py

def is_same_except_arg_names(self, extrascrub, args_and_sig, attrs):
     es1 = IR_VALUE_RE.sub(repl_arg_names, extrascrub)
     es0 = SCRUB_IR_COMMENT_RE.sub(r'', es0)
     es1 = SCRUB_IR_COMMENT_RE.sub(r'', es1)
     return es0 == es1
 
   def __str__(self):
     return self.scrub
 
+def get_failed_prefixes(func_dict):
+  # 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. Effectivelly, this prefix is unused and should
+  # be removed.
+  for prefix in func_dict:
+    if (not [fct for fct in func_dict[prefix] 
+         if func_dict[prefix][fct] is not None]):
+      yield prefix
+
+def warn_on_failed_prefixes(func_dict):
+  for prefix in get_failed_prefixes(func_dict):
+      warn('Prefix %s had conflicting output from different RUN lines for all functions' % (prefix,))
+
 def build_function_body_dictionary(function_re, scrubber, scrubber_args, raw_tool_output, prefixes, func_dict, func_order, verbose, record_args, check_attributes):
   for m in function_re.finditer(raw_tool_output):
     if not m:
       continue
     func = m.group('func')
     body = m.group('body')
     attrs = m.group('attrs') if check_attributes else ''
     # Determine if we print arguments, the opening brace, or nothing after the function name
if func.startswith('stress'):
       # We only use the last line of the function body for stress tests.
       scrubbed_body = '\n'.join(scrubbed_body.splitlines()[-1:])
     if verbose:
       print('Processing function: ' + func, file=sys.stderr)
       for l in scrubbed_body.splitlines():
         print('  ' + l, file=sys.stderr)
     for prefix in prefixes:
       if func in func_dict[prefix]:
-        if str(func_dict[prefix][func]) != scrubbed_body or (func_dict[prefix][func] and (func_dict[prefix][func].args_and_sig != args_and_sig or func_dict[prefix][func].attrs != attrs)):
-          if func_dict[prefix][func] and func_dict[prefix][func].is_same_except_arg_names(scrubbed_extra, args_and_sig, attrs):
+        if (func_dict[prefix][func] is None or
+            str(func_dict[prefix][func]) != scrubbed_body or
+            func_dict[prefix][func].args_and_sig != args_and_sig or
+                func_dict[prefix][func].attrs != attrs):
+          if (func_dict[prefix][func] is not None and
+              func_dict[prefix][func].is_same_except_arg_names(scrubbed_extra,
+                                                               args_and_sig,
+                                                               attrs)):
             func_dict[prefix][func].scrub = scrubbed_extra
             func_dict[prefix][func].args_and_sig = args_and_sig
             continue
           else:
-            if prefix == prefixes[-1]:
-              warn('Found conflicting asm under the same prefix: %r!' % (prefix,))
-            else:
-              func_dict[prefix][func] = None
-              continue
+            # This means a previous RUN line produced a body for this function
+            # that is different from the one produced by this current RUN line,
+            # so the body can't be common accross RUN lines. We use None to
+            # indicate that.
 
       func_dict[prefix][func] = function_body(scrubbed_body, scrubbed_extra, args_and_sig, attrs)
       func_order[prefix].append(func)
+  warn_on_failed_prefixes(func_dict)
 
 ##### Generator of LLVM IR CHECK lines
 
 SCRUB_IR_COMMENT_RE = re.compile(r'\s*;.*')
 
 # TODO: We should also derive check lines for global, debug, loop declarations, etc..
 
 class NamelessValue:

llvm/utils/update_test_prefix.py

with open(i, 'r+') as f:
             s = re.sub('\s+-?-check-prefix=CHECK[ \t]*\n', '\n', s)
         f.truncate(0)
         f.seek(0)
         f.write(s)
     if not r:
         t = re.search('Assertions have been autogenerated by (.*)', s)
         if t:
             s = os.popen('llvm/' + t.group(1) + ' ' + i + ' 2>&1').read()
-            if 'Found conflicting' in s:
+            if 'had conflicting output from different RUN lines for all functions' in s:
                 return -1
             s = os.popen('git diff ' + i).read()
             if re.search('\n(?:-+)\n', s) or re.search('\n[+-].*(?<!RUN):', s):
                 return 1
         return 0
     return remove_prefix(i, d+1)
 
 with ThreadPoolExecutor(max_workers=32) as e: