[PATCH] D149699: [InstCombine] Improve bswap optimization

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:6032
+    // e.g. bswap(logic_op(bswap(x), bswap(y)))
+    //  --> bswap(bswap(logic_op(x, y)))
     if (match(Op0, m_BSwap(m_Value(X))))
----------------
I don't think we need this comment... This is not the kind of optimization that needs extra justification in the code.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:10756
 
+  // fold (bswap (logic_op(bswap(x),y))) -> logic_op(x,bswap(y))
+  // Need to ensure logic_op and bswap(x) doesn't have other uses
----------------
goldstein.w.n wrote:
> Likewise a helper here so it can be easily extended to handle bitreverse/any other intrinsics that it applies to.
> Also, there are no tests for the DAGCombiner changes at the moment.
In any case, this should be part of a separate patch, not mixed with InstCombine changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149699