[PATCH] D149577: [InstCombine] Improve bswap optimization

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:1682
+    // bswap(logic_op(bswap(x), y)) --> logic_op(x, bswap(y))
+    if (match(IIOperand, m_OneUse(m_BitwiseLogic(m_Value(X), m_Value(Y))))) {
+      Value *OldSwap;
----------------
Can you move this to a helper function that takes an Intrisinic ID so we can reuse for `bitreverse`?


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:1693
+        Value *NewSwap = Builder.CreateUnaryIntrinsic(Intrinsic::bswap, Y);
+        return BinaryOperator::Create(Op, OldSwap, NewSwap);
+      } else if (match(Y, m_BSwap(m_Value(OldSwap))) && Y->hasOneUse()) {
----------------
goldstein.w.n wrote:
> I don't see how this is correct if we only have 1 bswap:
> https://alive2.llvm.org/ce/z/zBErQk
> 
> Also this apples for `bitreverse` as well.
> I don't see how this is correct if we only have 1 bswap:
> https://alive2.llvm.org/ce/z/zBErQk
> 
> Also this apples for `bitreverse` as well.

Nevermind, saw this as standalone, missed it was in the bswap case.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149577