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

Thu Dec 10 02:06:32 PST 2020

lebedev.ri added a comment.

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

> In D92965#2444565 <https://reviews.llvm.org/D92965#2444565>, @mtrofin wrote:
>
>> In D92965#2444553 <https://reviews.llvm.org/D92965#2444553>, @pengfei wrote:
>>
>>> I left about 12 tests when I fixed the prefixes issues using the script, because the changes on prefixes will resulted in conflict when re-generating them. I suspected if it is a bug in update_llc_test_checks.py, but I didn't find time to look through. Maybe we can add `--allow-unused-prefixes=false` for these failed tests as a workaround?
>>
>> I actually just undid the re-generation changes, leaving just the unused prefix removal. I'm not sure what the pros/cons to re-generation would be - my reasoning was about "keeping this change scoped". The tests passed, though with a FileCheck with the flag flipped to not allow unused prefixes.
>
> I'm afraid when pepole change something and need to update the tests, the conflict will confuse them and may spend more time to know what happened. At least we need to remove
>
>   ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py

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

> , though I think adding `--allow-unused-prefixes=false` seems better.

I think you meant `--allow-unused-prefixes=true`.

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

> I'd like to see other reviewers opinions.

+1, this is precisely why i think this whole `--allow-unused-prefixes=false`-by-default endeavor is a horrible idea.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92965