[PATCH] D149783: [DAGCombiner] Add bswap(logic_op(bswap(x), y)) optimization

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:9987
+    // Need to ensure logic_op and bswap/bitreverse(x) doesn't have other uses
+    if (OldLHS.getOpcode() == Opcode && OldLHS.hasOneUse()) {
+      SDValue NewBitReorder = DAG.getNode(Opcode, DL, VT, OldRHS);
----------------
RKSimon wrote:
> OldLHS might be from a node with multiple results - you need OldLHS->hasOneUse() (same for OldRHS below)
Thats already there in: `if (OldLHS.getOpcode() == Opcode && OldLHS.hasOneUse()) {` no?

Although I realized we will miss the following case:
`(bswap (logic (bswap X), (bswap Y))` -> `(logic X, Y)`
if `(bswap X)` and / or `(bswap Y)` are multi-use.

Even if both arms are mult-use we still save an instruction doing this optimizations.
Maybe there should be a check if both arms match opcode, and if so, to ignore
multiuse check?


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

https://reviews.llvm.org/D149783