[PATCH] D53710: [FileCheck] Warn if a prefix is only used in LABEL checks

Joel E. Denny via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 26 07:07:47 PDT 2018


jdenny added a comment.

In https://reviews.llvm.org/D53710#1277094, @SjoerdMeijer wrote:

> Thanks for looking at this! Answering this one first:
>
> > Have you run existing test suites to see how many warnings you get?
>
> I just did, and it came back with some interesting results. For example `test/CodeGen/ARM/Windows/mangling.ll`:
>
>   ; RUN: llc -mtriple=thumbv7-windows -mcpu=cortex-a9 -o - %s | FileCheck %s
>   define void @function() nounwind {
>   entry:
>     ret void
>   }
>   ; CHECK-LABEL: function
>    
>
> This shows that just checking the label is actually a use-case, which is why I thought a warning would be better; looks like we can't promote it to an error (which is okay I think).


Why isn't that just CHECK?  Is that a legitimate use case?

> Then because of this warning, I found:
> 
> - one of our own recently added test cases `test/CodeGen/ARM/arm-cgp-calls.ll` that was horribly broken in different ways; but it mainly wasn't checking anything because all label prefixes where wrong. I put up the fix for review in https://reviews.llvm.org/D53746.
> - another broken test case `test/CodeGen/ARM/inlineasm-X-allocation.ll`, for which I put up a fix for review here https://reviews.llvm.org/D53748.
> - Dubious test cases that only checks labels, but really should test other things too:
>   - `test/CodeGen/ARM/2011-11-07-PromoteVectorLoadStore.ll`,
>   - `test/CodeGen/ARM/2011-11-09-BitcastVectorDouble.ll`,
>   - `test/CodeGen/ARM/fast-isel-update-valuemap-for-extract.ll`
>   - `test/CodeGen/ARM/thumb2-size-reduction-internal-flags.ll`
> 
> I think this shows the benefit of adding this to FileCheck, and I've only looked at the tests in the `CodeGen/ARM` directory.

Nice.


https://reviews.llvm.org/D53710





More information about the llvm-commits mailing list