[PATCH] D111804: [InstCombine] Don't combine CmpInst that used for verify the legality of the lshr

[Tiehu Zhang via Phabricator via llvm-commits]

TiehuZhang added a comment.

In D111804#3065956 <https://reviews.llvm.org/D111804#3065956>, @craig.topper wrote:

> In D111804#3065924 <https://reviews.llvm.org/D111804#3065924>, @TiehuZhang wrote:
>
>> In D111804#3065906 <https://reviews.llvm.org/D111804#3065906>, @craig.topper wrote:
>>
>>> If an input to an `or` instruction is poison, the output of the `or` is poison regardless of whether the other operand is true when the poison result occurs.
>>>
>>> If an incorrect optimization is occurring it must be where the `or` is created.
>>
>> Thanks for your review! @craig.topper. Do you mean that we should prevent this pattern from being generated, rather than remedying it after the pattern is generated?  Actually, the issue I met started from D95959 <https://reviews.llvm.org/D95959>. That optimization does not seem to apply to all scenarios, such as those where the shift operand is negative. In the case I present, this optimization results in a miscalculation of the KnownBit, which in turn results in a false transformation.
>
> The transform is correct based on the semantics of llvm's `or` instruction. The `or` instruction in llvm does not have short circuit evaluation. It can't block the spread of poison. The IR that has the semantics you want is
>
>   define i32 @cmp_lshr_cmp(i32 %0) {
>   entry:
>     %cmp = icmp slt i32 %0, 0
>     %shr = lshr i32 7, %0
>     %cmp1 = icmp sgt i32 %0, %shr
>     %or.cond = select i1 %cmp, i1 true, i1 %cmp1
>     br i1 %or.cond, label %cond.end, label %cond.false
>   
>   cond.false:                                       ; preds = %entry
>     ret i32 1
>   
>   cond.end:                                         ; preds = %entry, %cond.false
>     ret i32 0
>   }
>
> Using a select instruction instead of an `or` prevents the %cmp1 value from being used if %cmp is true. There was previously an incorrect transform that would turn such a select into an `or`, but it was disabled in https://reviews.llvm.org/D101191

OK, I get it. I'll try to find out why the `or` is still generated. Thanks very much @craig.topper


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111804