[llvm-dev] [RFC] FileCheck: (dis)allowing unused prefixes

Wang, Pengfei via llvm-dev
2020-11-16T02:30:53Z

Hi,

I wrote a script to help removing unused trivial prefixes. It is located in
llvm/utils/update_test_prefix.py.
It is a rough one and based on problems I met. It also highly relies on the
output of llvm-lit and update tools. So carefully verification on its changes is
MUST.

Here's the usage of it:
1. turn allow-unused-prefixes to false and built it: sed -i
'/allow-unused-prefixes/s/true/false/'
llvm/utils/FileCheck/FileCheck.cpp;
2. cd llvm-project;
3. make sure llvm-lit and update_llc_test_checks.py or other update tools are
available;
4. llvm/utils/update_test_prefix.py test_file_you_want_to_update.xxx
5. you can also put them in a file and cat update_tests.txt | xargs
llvm/utils/update_test_prefix.py

The script removes unused prefixes and verifies those autogenerated tests. It
reports "Failed" if there's conflicting in regeneration and
"Changed" if there're changes in the check contents. There's a
known issue if the prefix is not connected by "=", e.g.
"--check-prefix X86". It will be put into "Aborted". Please
pay attention for tests in these lists.
Wish the script would help you😊

Thanks
Pengfei

-----Original Message-----
From: Fāng-ruì Sòng <maskray at google.com>
Sent: Friday, November 6, 2020 3:50 AM
To: Matt Arsenault <Matthew.Arsenault at amd.com>; Alexey.Bataev at
ibm.com; Tim Northover <t.p.northover at gmail.com>; Wang, Pengfei
<pengfei.wang at intel.com>
Cc: Mircea Trofin <mtrofin at google.com>; David Blaikie <dblaikie at
gmail.com>; LLVM Dev <llvm-dev at lists.llvm.org>
Subject: Re: [llvm-dev] [RFC] FileCheck: (dis)allowing unused prefixes

On Thu, Nov 5, 2020 at 11:36 AM David Blaikie via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
>
>
>
> On Thu, Nov 5, 2020 at 10:46 AM Mircea Trofin <mtrofin at google.com>
wrote:
>>
>>
>>
>> On Thu, Nov 5, 2020 at 10:40 AM David Blaikie <dblaikie at
gmail.com> wrote:
>>>
>>>
>>>
>>> On Thu, Nov 5, 2020 at 7:30 AM Mircea Trofin via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>>>>
>>>> There are currently 1350 owner-less failures in the
spreadsheet. These seem to be the larger areas there.
>>>>
>>>> If you see an area you have ownership or expertise in, please
sign up for fixing the tests by Monday, Nov. 9.
>>>>
>>>> Otherwise, I will "blanket-add"
--allow-unused-prefixes=true to the remaining failing tests.
>>>
>>>
>>> If/when you do that, probably worth adding a comment at each site
to clarify that this was added automatically, not vetted/intentionally added by
a human. Something like "// FIXME: Verify that unused prefixes are used
intentionally" or the like.
>>
>> Ack. or, we can grep for -allow-unused-prefixes=true, wdyt?
>
>
> Not sure I understand who/when they would grep for that?
>
> I was suggesting adding an explicit "this use of
--allow-unused-prefixes=true hasn't been confirmed as intentional" so
that the backwards compatibility cases can be distinguished from the intentional
cases when someone is reading the test case, rather than puzzling over why this
flag was added (which looks intentional) though the unused prefix may not make
sense in that particular test. It'll make it easier in the future when
someone does look at the test for them to not feel like they're being
implicitly told "this use of unused prefixes is intentional" (by the
presence of an explicit flag requesting such support) while staring at the test
and not being able to see why someone would've done that intentionally.

Directly CCing some folks who can fix or find people to fix some directories
(*/AMDGPU, */X86, */OpenMP, */AArch64) on
https://docs.google.com/spreadsheets/d/1o6q3XH1n3DDyyccnYZ_kVfhFbTDzC_S09e973_cwYuw/edit#gid=0
After these big directories are cleaned up, the remaining tests should be
manageable in amount.

How to reproduce:

sed -i '/allow-unused-prefixes/s/true/false/'
llvm/utils/FileCheck/FileCheck.cpp
git update-index --assume-unchanged llvm/utils/FileCheck/FileCheck.cpp
ninja check-llvm   # or check-clang ...




>>>
>>>
>>>>
>>>>
>>>>
>>>> On Fri, Oct 30, 2020 at 12:48 PM Mircea Trofin <mtrofin at
google.com> wrote:
>>>>>
>>>>> An update: as of 871d658c9ceb, the flag is now available,
if folks need to use it.
>>>>>
>>>>> On Thu, Oct 29, 2020 at 10:28 AM Mircea Trofin <mtrofin
at google.com> wrote:
>>>>>>
>>>>>> Hello all,
>>>>>>
>>>>>> TL;DR; if you used FileCheck --check-prefixes and you
missed (misspelled, for instance) one of the prefixes in your test, FileCheck
silently ignores that and the test passes.
>>>>>>
>>>>>> 1579 tests have this property.
>>>>>>
>>>>>>
>>>>>> The details
>>>>>> ========>>>>>> Please refer to
https://reviews.llvm.org/D90281 and the
>>>>>> discussion there for more details (make sure you open
"older
>>>>>> changes" for full context)
>>>>>>
>>>>>> The problem is covered by the TL;DR;.
>>>>>>
>>>>>> The proposal is to add an explicit flag to FileCheck,
--allow-unused-prefixes, to indicate whether the current behavior is intended
(for instance, jdoerfert contributed a scenario where that is the case).
>>>>>>
>>>>>> We want the default behavior to be 'strict',
i.e. --allow-unused-prefixes=false. Doing that right now would lead to 1500 test
failures.
>>>>>>
>>>>>> To get there (thanks, maskray, for suggestion), we
propose we:
>>>>>> * land D90281 where the flag is introduced, but is
flipped to
>>>>>> match today's behavior
>>>>>> * employ a 'busy beavers' approach, where test
maintainers patch their tests:
>>>>>>   - either leveraging the flag, to explicitly indicate
that unused prefixes is intended (i.e. add --allow-unused-patches=true); or
>>>>>>   - fix the test (e.g. maybe there was a misspelling
issue/omission/etc).
>>>>>>
>>>>>> A spreadsheet with the failing tests is available here
[1].
>>>>>>
>>>>>> The request to the community members is to please sign
up for their respective area in the spreadsheet, and then mark it completed when
that's the case (yes/no in the respective column).
>>>>>>
>>>>>> When all the tests are fixed, we will then flip
--allow-unused-prefixes to false by default.
>>>>>>
>>>>>> Meanwhile, please consider leveraging the flag
explicitly when you author new tests that use --check-prefixes. That can be then
cleaned up easily after we switch to the 'strict' behavior.
>>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>> [1]
>>>>>>
https://docs.google.com/spreadsheets/d/1o6q3XH1n3DDyyccnYZ_kVfhFb
>>>>>> TDzC_S09e973_cwYuw/edit?usp=sharing
>>>>
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> llvm-dev at lists.llvm.org
>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev



--
宋方睿