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

[Danila Kutenin via Phabricator via llvm-commits]

danlark added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:274-275
+  if (I.hasNoSignedWrap()) {
+    const APInt *V;
+    if (match(C1, m_APInt(V)) && *V != V->getBitWidth() - 1)
+      Shl->setHasNoSignedWrap();
----------------
lebedev.ri wrote:
> danlark wrote:
> > lebedev.ri wrote:
> > > This isn't sound.
> > > We want to not propagate `NSW` if *any* lane matches this predicate, not just if they all do..
> > > So you want
> > > ```
> > > if (I.hasNoSignedWrap() && !C1.isNotMinSignedValue())
> > >   Shl->setHasNoSignedWrap();
> > > ```
> > Yeah, I agree. Maybe you meant !cast<Constant>(Op1)->**isMinSignedValue()**
> I'm very sure i didn't - that has the same problem of only checking for splat INT_MIN while we want to check if any channel is INT_MIN.
I believe for now it is correct, C1 is a logarithm of Op1, if Op1 is INT_MIN, then it should not have nsw. So, if all are not int_min, it will be propagated, otherwise, it will not.

```
define i32 @shift_if_power2_nuw_nsw_min(i32 %x, i1 %cond) {
  %sel = select i1 %cond, i32 2, i32 -2147483648
  %r = mul nuw nsw i32 %sel, %x
  ret i32 %r
}

===>

define i32 @shift_if_power2_nuw_nsw_min(i32 %x, i1 %cond) {
  %sel = select i1 %cond, i32 1, i32 31
  %r =  shl nuw i32 i32 %x, %sel
  ret i32 %r
}
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71568