[PATCH] D71568: [InstCombine] `select + mul` -> `select + shl` with power of twos.

[Danila Kutenin via Phabricator via llvm-commits]

danlark marked 4 inline comments as done.
danlark added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:267-269
+  Constant *C1 = getLogBase2(Op0->getType(), cast<Constant>(Op1));
+  if (!C1)
+    llvm_unreachable("Failed to constant fold mul -> logbase2");
----------------
lebedev.ri wrote:
> danlark wrote:
> > lebedev.ri wrote:
> > > What happens with undef lanes?
> > > (they should become `0`, do not keep them as `undef`.
> > > there was some helper for that, can't find it now "replace undef values with")
> > They will remain undef as in division, why is 0 better?
> Keeping `undef` in this situation is (a very common) miscompilation.
> ```
> define <2 x i8> @shift_if_different_lanes_undef_vector(<2 x i8> %px, i1 %cond) {
> %0:
>   %sel = select i1 %cond, <2 x i8> { 16, undef }, <2 x i8> { undef, 32 }
>   %r = mul <2 x i8> %px, %sel
>   ret <2 x i8> %r
> }
> =>
> define <2 x i8> @shift_if_different_lanes_undef_vector(<2 x i8> %px, i1 %cond) {
> %0:
>   %sel = select i1 %cond, <2 x i8> { 4, undef }, <2 x i8> { undef, 5 }
>   %r = shl <2 x i8> %px, %sel
>   ret <2 x i8> %r
> }
> Transformation doesn't verify!
> ERROR: Target is more poisonous than source
> 
> Example:
> <2 x i8> %px = < undef, undef >
> i1 %cond = undef
> 
> Source:
> <2 x i8> %sel = < undef, undef >
> <2 x i8> %r = < undef, undef >
> 
> Target:
> <2 x i8> %sel = < #x04 (4), #x08 (8) >
> <2 x i8> %r = < #x00 (0)        [based on undef value], poison >
> Source value: < undef, undef >
> Target value: < #x00 (0), poison >
> ```
> ```
> define <2 x i8> @shift_if_different_lanes_undef_vector(<2 x i8> %px, i1 %cond) {
> %0:
>   %sel = select i1 %cond, <2 x i8> { 16, undef }, <2 x i8> { undef, 32 }
>   %r = mul <2 x i8> %px, %sel
>   ret <2 x i8> %r
> }
> =>
> define <2 x i8> @shift_if_different_lanes_undef_vector(<2 x i8> %px, i1 %cond) {
> %0:
>   %sel = select i1 %cond, <2 x i8> { 4, 0 }, <2 x i8> { 0, 5 }
>   %r = shl <2 x i8> %px, %sel
>   ret <2 x i8> %r
> }
> Transformation seems to be correct!
> ```
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71568