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

Fri Feb 5 07:57:02 PST 2021

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/c37c651c/attachment.html>