[PATCH] D123453: [InstCombine] Fold mul nuw+lshr exact to a single multiplication when the latter is a factor

[chenglin.bi via Phabricator via llvm-commits]

bcl5980 added a comment.

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

> In D123453#3441326 <https://reviews.llvm.org/D123453#3441326>, @bcl5980 wrote:
>
>> In D123453#3441216 <https://reviews.llvm.org/D123453#3441216>, @lebedev.ri wrote:
>>
>>> For example, what about this:
>>> ~~https://alive2.llvm.org/ce/z/aov-GB <https://alive2.llvm.org/ce/z/aov-GB>~~
>>> https://alive2.llvm.org/ce/z/ox4wAt (no need for `exact`, only `nuw`)
>>
>>
>>
>>   %t0 = lshr i8 %C1, %C2
>>   %t1 = shl i8 %t0, %C2
>>   %precond = icmp eq i8 %t1, %C1
>>   call void @llvm.assume(i1 %precond)
>>
>> This is the proof of exact. We can't remove exact check in the code. Or we need to do something like:
>>
>>   if (Op0->hasOneUse()) {
>>     APInt NewMulC = MulC->lshr(ShAmtC);
>>     if (MulC->eq(NewMulC.shl(ShAmtC)))
>>       return BinaryOperator::CreateNUWMul(X, ConstantInt::get(Ty, NewMulC));
>>   }
>
> Couldn't the exact be set because the LHS of the multiply is known to have trailing zero bits and have nothing to do with the trailing zeros of the constant on the RHS. Would the transform still be valid in that case?

Thanks for the finding. That's what I haven't expect this case. So we should use the check if (MulC->eq(NewMulC.shl(ShAmtC))) to avoid mul's LHS involve exact.


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

https://reviews.llvm.org/D123453