[PATCH] D122835: [SCEV] Fix a bug that caused an invalid assertion.

YangguangLi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 1 13:40:25 PDT 2022


Yangguang added a comment.

In D122835#3420698 <https://reviews.llvm.org/D122835#3420698>, @lebedev.ri wrote:

> In D122835#3420684 <https://reviews.llvm.org/D122835#3420684>, @Yangguang wrote:
>
>> In D122835#3419988 <https://reviews.llvm.org/D122835#3419988>, @lebedev.ri wrote:
>>
>>> Test?
>>
>> I can't create a test that fails the original assertion. When the `ExitCond` is NOT a `BinaryOperator` (which means it's in select form/short-circuit form), the original assert will always pass and we won't need to do the next two checks. But according to the comment above the assertion and the original design for the assertion, we should be checking the next two conditions when the `ExitCond` is in select form. I updated the summary, please refer for more info.
>
> I mean the test on which the unfixed assertion fires.
> Though, i suspect that the assertion is still wrong, and the fix should be to just drop the `!isa<BinaryOperator>(ExitCond) || ` part.

Fixed as suggested and changed the comment accordingly. Please check if it make sense.


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

https://reviews.llvm.org/D122835



More information about the llvm-commits mailing list