[PATCH] D92965: [NFC] Remove unused prefixes in llvm/test/CodeGen/X86

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added a comment.

In D92965#2445701 <https://reviews.llvm.org/D92965#2445701>, @pengfei wrote:

>> I think you meant `--allow-unused-prefixes=true`.
>
> Yes, that's what I meant. Thanks for correct me.

I was not familiar with update_llc_test_checks.py before this point, and, after digging a bit more, I understand @lebedev.ri 's concern - looks like quite a few (3K) of tests in CodeGen (14K or so) use it. So I wanted to understand why removing a prefix used nowhere in the test lead to an incorrect rewrite of some assertions.

I believe I found the cause - ptal D93078 <https://reviews.llvm.org/D93078>.

Separately, am I understanding update_llc_test.checks.py's usage accurately - IIUC, it's basically this:

- write new RUN line. Add, to FileCheck, a list of prefixes to check
- write functions
- run update_llc_test.checs.py => it will add the pertinent asserts to the function(s) added above

I can understand one may add a larger list of labels than necessary when initially authoring a RUN line. Apologies if I'm missing a pain point not obvious to me (given lack of familiarity) - I'd argue that tidying that at this point should be quite straight forward: should the list contain prefixes that end up not being used, FileCheck gives that sub-list; if the outcome is surprising to the developer (i.e. they expected a label be actually used), they get an earlier heads-up than needing to scroll down to where the asserts were inserted; in either case, the fix should be quick (it's in the current change); and the benefit is that what the developer intended is actually checked for.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92965/new/

https://reviews.llvm.org/D92965