[PATCH] D110752: [SimpleLoopUnswitch] Don't unswitch constant conditions

Arthur Eubanks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 30 14:55:34 PDT 2021


aeubanks added a comment.

In D110752#3035012 <https://reviews.llvm.org/D110752#3035012>, @DaniilSuchkov wrote:

> In D110752#3034962 <https://reviews.llvm.org/D110752#3034962>, @aeubanks wrote:
>
>> rebasing this on D110751 <https://reviews.llvm.org/D110751> is confusing, especially since the test is XFAIL in that patch
>> I'd just add the test and run update_test_checks.py on it in this patch
>>
>> then just add the asserts in D110751 <https://reviews.llvm.org/D110751> in don't touch any test cases in that patch
>
> Well, D110751 <https://reviews.llvm.org/D110751> turns the miscompile into and assertion failure, and with this patch instead of assertion failure we'll get a completely valid result.
> I think this sequence of changes makes sense.

Probably a matter a preference, but I think we should fix the issue first. I'm not a fan of XFAIL tests. You could even merge the two patches into one.
Any other preferences from other reviewers?


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

https://reviews.llvm.org/D110752



More information about the llvm-commits mailing list