[PATCH] D92965: [NFC] Remove unused prefixes in llvm/test/CodeGen/X86
Roman Lebedev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 10 02:04:08 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
>
> , though I think adding `--allow-unused-prefixes=false` seems better.
> I'd like to see other reviewers opinions.
+1, this is precisely why i think this whole `--allow-unused-prefixes=true`-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
More information about the llvm-commits
mailing list