[llvm-dev] Conflicting check prefix detection not working in update_llc_test_checks.py?

Sanjay Patel via llvm-dev llvm-dev at lists.llvm.org
Fri Feb 5 08:50:30 PST 2021 

Yes, warning only if a function has no asserts sounds like it would restore
the previous behavior?

If I modify the example test file with a common prefix like this, then I
agree with Pengfei that there should be no warnings - all functions have
the expected (either common or unique) assertions:

diff --git a/llvm/test/CodeGen/RISCV/alu32.ll
b/llvm/test/CodeGen/RISCV/alu32.ll
index 975716e53c04..bf74ef5eab6f 100644
--- a/llvm/test/CodeGen/RISCV/alu32.ll
+++ b/llvm/test/CodeGen/RISCV/alu32.ll
@@ -1,292 +1,252 @@
 ; NOTE: Assertions have been autogenerated by
utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
-; RUN:   | FileCheck %s -check-prefix=RV32I
+; RUN:   | FileCheck %s --check-prefixes=CHECK,RV32I
 ; RUN: llc -mtriple=riscv64 -verify-machineinstrs < %s \
-; RUN:   | FileCheck %s -check-prefix=RV64I
+; RUN:   | FileCheck %s --check-prefixes=CHECK,RV64I

On Fri, Feb 5, 2021 at 10:57 AM Mircea Trofin <mtrofin at google.com> wrote:

> Thanks - so would it be then desirable to just warn *if* a function ends
> up stripped of any assertions? I.e. say you have 2 functions, f1 and f2.
> First RUN line produces an output for both for prefixes "P1 and P2". Second
> RUN line produces a conflicting output for f1 for P1 (f2 is OK). Third RUN
> line produces a conflicting output for f1 for P2. Result: f1 has no asserts
> => we warn.
>
> On Thu, Feb 4, 2021 at 11:22 PM Wang, Pengfei <pengfei.wang at intel.com>
> wrote:
>
>> It looks to me the "Function ..." warnings are unnecessary. The test has
>> been updated as expected.
>> I agreed with Craig. I was thinking of this pain point when I removed
>> those used prefixes.
>> I believe it's of little account for experienced users like Craig, Simon
>> and Sanjay who adding finer grained prefixes for X86.
>> But I'm worry about junior user or developers form other targets who
>> accidently changes X86 tests.
>>
>> Thanks
>> Pengfei
>>
>> -----Original Message-----
>> From: Fāng-ruì Sòng <maskray at google.com>
>> Sent: Friday, February 5, 2021 2:34 PM
>> To: Craig Topper <craig.topper at gmail.com>
>> Cc: Mircea Trofin <mtrofin at google.com>; llvm-dev <llvm-dev at lists.llvm.org>;
>> Simon Pilgrim <llvm-dev at redking.me.uk>; Sanjay Patel <
>> spatel at rotateright.com>; Wang, Pengfei <pengfei.wang at intel.com>
>> Subject: Re: [llvm-dev] Conflicting check prefix detection not working in
>> update_llc_test_checks.py?
>>
>> Add back dropped CC: Simon and Sanjay.
>>
>> They use update_llc_test_checks.py heavily, perhaps, they can help? :)
>>
>> On Wed, Feb 3, 2021 at 11:25 AM Craig Topper via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>> >
>> > Maybe. I'm not quite sure what you mean by "functions with no asm".
>> >
>> > X86 especially has a lot of tests with a half dozen or more RUN lines.
>> There is a common prefix for all RUN lines or at least all AVX/AVX512 run
>> lines and then successively more precise check prefixes grouping smaller
>> and smaller subsets of RUN lines. The goal is to minimize the number of
>> check lines in the file by grouping as much as possible. But allowing
>> precise checks for individual functions that have different behavior on
>> many targets.
>> >
>> > Since we can't have unused prefixes in the test files any more we might
>> lose some of the finer grained check prefixes if multiple RUN lines produce
>> the same results for all functions today. A future change may break that
>> and require finer grained prefixes to be added. That's the important case
>> to warn about since it requires the user to change the RUN lines.
>> >
>> > ~Craig
>> >
>> >
>> > On Tue, Feb 2, 2021 at 9:39 PM Mircea Trofin <mtrofin at google.com>
>> wrote:
>> >>
>> >> I see, so maybe there are some levels of warning to observe -
>> "everything", functions with no asm, prefixes with no use. And maybe the
>> default is the second?
>> >>
>> >> On Tue, Feb 2, 2021, 21:35 Craig Topper <craig.topper at gmail.com>
>> wrote:
>> >>>
>> >>> Hi Mircea,
>> >>>
>> >>> It looks like the script is now reporting warnings even when there is
>> a set of prefixes to update the test for all functions. For example, if you
>> add -check-prefixes=CHECK,RV32I to the first command in alu32.ll and
>> -check-prefixes=CHECK,RV64I to the second you'll get this, but it looks
>> like there's a valid solution for the test and no user intervention is
>> required.
>> >>>
>> >>> WARNING: Function slti had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function sltiu had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function srli had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function srai had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function add had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function sub had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function sll had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function slt had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function sltu had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function srl had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function sra had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>>
>> >>> ~Craig
>> >>>
>> >>>
>> >>> On Mon, Feb 1, 2021 at 3:19 PM Craig Topper <craig.topper at gmail.com>
>> wrote:
>> >>>>
>> >>>> Yes, that matches my expectations. Thanks!
>> >>>>
>> >>>> ~Craig
>> >>>>
>> >>>>
>> >>>> On Mon, Feb 1, 2021 at 3:05 PM Mircea Trofin <mtrofin at google.com>
>> wrote:
>> >>>>>
>> >>>>> Indeed, we're now not output-ing the case where some functions have
>> conflicting asm, just the case when all functions lose their asm.
>> >>>>>
>> >>>>> I have a fix ready; to confirm, for this example (i.e. taking all
>> (both) the "--check-prefix"-es in alu32.ll), would this output match your
>> expectations?
>> >>>>>
>> >>>>> WARNING: Function slti had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function sltiu had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function srli had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function srai had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function add had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function sub had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function sll had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function slt had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function sltu had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function srl had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function sra had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>>
>> >>>>> On Mon, Feb 1, 2021 at 2:12 PM Mircea Trofin <mtrofin at google.com>
>> wrote:
>> >>>>>>
>> >>>>>> looking
>> >>>>>>
>> >>>>>> On Mon, Feb 1, 2021 at 2:11 PM Craig Topper <
>> craig.topper at gmail.com> wrote:
>> >>>>>>>
>> >>>>>>> update_llc_test_checks.py seems to not be telling me about
>> assembly that differs under the same prefix anymore.
>> >>>>>>>
>> >>>>>>> An easy way to see this is to just remove the --check-prefix from
>> test/CodeGen/RISCV/alu32.ll and run the script. You'll get no error about
>> conflicts. And if you look at the resulting file only some functions will
>> have been updated to use CHECK as the prefix.
>> >>>>>>>
>> >>>>>>> Reverting some commits to update_llc_test_checks.py suggest this
>> >>>>>>> may have been broken by e2dc306b1ac71258e6ce40a66e778527f282c355
>> >>>>>>> [utils] Fix UpdateTestChecks case where 2 runs differ for last
>> >>>>>>> label
>> >>>>>>>
>> >>>>>>> ~Craig
>> >
>> > _______________________________________________
>> > LLVM Developers mailing list
>> > llvm-dev at lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>>
>>
>> --
>> 宋方睿
>>
>
On Fri, Feb 5, 2021 at 10:57 AM Mircea Trofin <mtrofin at google.com> wrote:

> Thanks - so would it be then desirable to just warn *if* a function ends
> up stripped of any assertions? I.e. say you have 2 functions, f1 and f2.
> First RUN line produces an output for both for prefixes "P1 and P2". Second
> RUN line produces a conflicting output for f1 for P1 (f2 is OK). Third RUN
> line produces a conflicting output for f1 for P2. Result: f1 has no asserts
> => we warn.
>
> On Thu, Feb 4, 2021 at 11:22 PM Wang, Pengfei <pengfei.wang at intel.com>
> wrote:
>
>> It looks to me the "Function ..." warnings are unnecessary. The test has
>> been updated as expected.
>> I agreed with Craig. I was thinking of this pain point when I removed
>> those used prefixes.
>> I believe it's of little account for experienced users like Craig, Simon
>> and Sanjay who adding finer grained prefixes for X86.
>> But I'm worry about junior user or developers form other targets who
>> accidently changes X86 tests.
>>
>> Thanks
>> Pengfei
>>
>> -----Original Message-----
>> From: Fāng-ruì Sòng <maskray at google.com>
>> Sent: Friday, February 5, 2021 2:34 PM
>> To: Craig Topper <craig.topper at gmail.com>
>> Cc: Mircea Trofin <mtrofin at google.com>; llvm-dev <llvm-dev at lists.llvm.org>;
>> Simon Pilgrim <llvm-dev at redking.me.uk>; Sanjay Patel <
>> spatel at rotateright.com>; Wang, Pengfei <pengfei.wang at intel.com>
>> Subject: Re: [llvm-dev] Conflicting check prefix detection not working in
>> update_llc_test_checks.py?
>>
>> Add back dropped CC: Simon and Sanjay.
>>
>> They use update_llc_test_checks.py heavily, perhaps, they can help? :)
>>
>> On Wed, Feb 3, 2021 at 11:25 AM Craig Topper via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>> >
>> > Maybe. I'm not quite sure what you mean by "functions with no asm".
>> >
>> > X86 especially has a lot of tests with a half dozen or more RUN lines.
>> There is a common prefix for all RUN lines or at least all AVX/AVX512 run
>> lines and then successively more precise check prefixes grouping smaller
>> and smaller subsets of RUN lines. The goal is to minimize the number of
>> check lines in the file by grouping as much as possible. But allowing
>> precise checks for individual functions that have different behavior on
>> many targets.
>> >
>> > Since we can't have unused prefixes in the test files any more we might
>> lose some of the finer grained check prefixes if multiple RUN lines produce
>> the same results for all functions today. A future change may break that
>> and require finer grained prefixes to be added. That's the important case
>> to warn about since it requires the user to change the RUN lines.
>> >
>> > ~Craig
>> >
>> >
>> > On Tue, Feb 2, 2021 at 9:39 PM Mircea Trofin <mtrofin at google.com>
>> wrote:
>> >>
>> >> I see, so maybe there are some levels of warning to observe -
>> "everything", functions with no asm, prefixes with no use. And maybe the
>> default is the second?
>> >>
>> >> On Tue, Feb 2, 2021, 21:35 Craig Topper <craig.topper at gmail.com>
>> wrote:
>> >>>
>> >>> Hi Mircea,
>> >>>
>> >>> It looks like the script is now reporting warnings even when there is
>> a set of prefixes to update the test for all functions. For example, if you
>> add -check-prefixes=CHECK,RV32I to the first command in alu32.ll and
>> -check-prefixes=CHECK,RV64I to the second you'll get this, but it looks
>> like there's a valid solution for the test and no user intervention is
>> required.
>> >>>
>> >>> WARNING: Function slti had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function sltiu had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function srli had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function srai had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function add had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function sub had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function sll had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function slt had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function sltu had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function srl had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>> WARNING: Function sra had conflicting output from different RUN
>> >>> lines for prefix CHECK
>> >>>
>> >>>
>> >>> ~Craig
>> >>>
>> >>>
>> >>> On Mon, Feb 1, 2021 at 3:19 PM Craig Topper <craig.topper at gmail.com>
>> wrote:
>> >>>>
>> >>>> Yes, that matches my expectations. Thanks!
>> >>>>
>> >>>> ~Craig
>> >>>>
>> >>>>
>> >>>> On Mon, Feb 1, 2021 at 3:05 PM Mircea Trofin <mtrofin at google.com>
>> wrote:
>> >>>>>
>> >>>>> Indeed, we're now not output-ing the case where some functions have
>> conflicting asm, just the case when all functions lose their asm.
>> >>>>>
>> >>>>> I have a fix ready; to confirm, for this example (i.e. taking all
>> (both) the "--check-prefix"-es in alu32.ll), would this output match your
>> expectations?
>> >>>>>
>> >>>>> WARNING: Function slti had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function sltiu had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function srli had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function srai had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function add had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function sub had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function sll had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function slt had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function sltu had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function srl had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>> WARNING: Function sra had conflicting output from different RUN
>> >>>>> lines for prefix CHECK
>> >>>>>
>> >>>>> On Mon, Feb 1, 2021 at 2:12 PM Mircea Trofin <mtrofin at google.com>
>> wrote:
>> >>>>>>
>> >>>>>> looking
>> >>>>>>
>> >>>>>> On Mon, Feb 1, 2021 at 2:11 PM Craig Topper <
>> craig.topper at gmail.com> wrote:
>> >>>>>>>
>> >>>>>>> update_llc_test_checks.py seems to not be telling me about
>> assembly that differs under the same prefix anymore.
>> >>>>>>>
>> >>>>>>> An easy way to see this is to just remove the --check-prefix from
>> test/CodeGen/RISCV/alu32.ll and run the script. You'll get no error about
>> conflicts. And if you look at the resulting file only some functions will
>> have been updated to use CHECK as the prefix.
>> >>>>>>>
>> >>>>>>> Reverting some commits to update_llc_test_checks.py suggest this
>> >>>>>>> may have been broken by e2dc306b1ac71258e6ce40a66e778527f282c355
>> >>>>>>> [utils] Fix UpdateTestChecks case where 2 runs differ for last
>> >>>>>>> label
>> >>>>>>>
>> >>>>>>> ~Craig
>> >
>> > _______________________________________________
>> > LLVM Developers mailing list
>> > llvm-dev at lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>>
>>
>> --
>> 宋方睿
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210205/6d839e02/attachment.html>

More information about the llvm-dev mailing list