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

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D110752#3035034 <https://reviews.llvm.org/D110752#3035034>, @aeubanks wrote:

> 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?

Agree, we should fix the issue first and then land the assert. We shouldn't add asserts that are known to fail in advance.


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

https://reviews.llvm.org/D110752