[PATCH] D117680: [InstCombine] Simplify bswap -> shift

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:1194
+
+    // bswap(x) -> shl(x) if x has at most 8 (byte) active bits
+    if (Known.countMaxActiveBits() <= 8)
----------------
craig.topper wrote:
> craig.topper wrote:
> > Can this go further.
> > 
> > Something like
> > 
> > ```
> > unsigned TZ = alignDown(Known.countMinTrailingZeros(), 8);
> > unsigned LZ = alignDown(Known.countMinLeadingZeros(), 8);
> > 
> > if (BitWidth - TZ - LZ == 8) {
> >   if ((BitWidth - LZ - 8) > TZ)
> >     ShiftRight by ((BitWidth - LZ - 8)  - TZ)
> >   else
> >     ShiftLeft by (TZ - (BitWidth - LZ - 8))
> > }
> > ```
> > 
> > Adapted from the SimplifyDemandedBits for bswap like https://reviews.llvm.org/D117508
> Oops that should be
> 
> ```
> unsigned TZ = alignDown(Known.countMinTrailingZeros(), 8);
> unsigned LZ = alignDown(Known.countMinLeadingZeros(), 8);
> 
> if (BitWidth - TZ - LZ == 8) {
>   if ((BitWidth - TZ - 8) > TZ)
>     ShiftRight by ((BitWidth - TZ - 8)  - TZ)
>   else
>     ShiftLeft by (TZ - (BitWidth - TZ - 8))
> }
> ```
Or even simpler.

```
unsigned TZ = alignDown(Known.countMinTrailingZeros(), 8);
unsigned LZ = alignDown(Known.countMinLeadingZeros(), 8);

if (BitWidth - TZ - LZ == 8) {
  if (LZ > TZ)
    ShiftRight by (LZ - TZ)
  else
    ShiftLeft by (TZ - LZ)
}
```

Wish I could amend or delete my previous comments.


================
Comment at: llvm/test/Transforms/InstCombine/bswap-fold.ll:421
+;
+  %2 = shl <2 x i64> %0, <i64 56, i64 undef>
+  %3 = call <2 x i64> @llvm.bswap.v2i64(<2 x i64> %2)
----------------
chfast wrote:
> Any `undef` shift index or `and` argument prevents this optimization. Is this expected?
I didn't think of that, but it is correct for computeKnownBits. The undef doesn't allow computeKnownBits to determine anything.

I don't think I would worry too much about it. @spatel or @lebedev.ri what do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117680