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

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.

In D117680#3259685 <https://reviews.llvm.org/D117680#3259685>, @chfast wrote:

> @spatel, can you also review this?

LGTM

> Should I land 2 commits: one adding tests, second with the implementation and test updates?

Yes, the patch to add tests should be labeled "NFC". Just in case this patch is reverted, the tests can remain in tree.



================
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)
----------------
craig.topper wrote:
> 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?
I doubt that vector bswap occurs enough to worry about, and the even rarer case with undefs shouldn't limit this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117680