[PATCH] D149699: [InstCombine] Improve bswap optimization

[Austin Chang via Phabricator via llvm-commits]

austin880625 added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:70
     // OP( BSWAP(x), BSWAP(y) ) -> BSWAP( OP(x, y) )
-    if (!OldLHS->hasOneUse() && !OldRHS->hasOneUse())
+    if (!OldLHS->hasOneUse() || !OldRHS->hasOneUse())
       return nullptr;
----------------
goldstein.w.n wrote:
> What is this change for?
if one of the operand has more than one use, then the transform will not reduce the number of instructions(can look at the `bs_and64_multiuse*` test case diff) and in DAGCombiner the same optimization also uses `||` instead of `&&`:

```
  // Unary ops: logic_op (bswap x), (bswap y) --> bswap (logic_op x, y)
  if (HandOpcode == ISD::BSWAP) {
    // If either operand has other uses, this transform is not an improvement.
    if (!N0.hasOneUse() || !N1.hasOneUse())
      return SDValue();
    SDValue Logic = DAG.getNode(LogicOpcode, DL, XVT, X, Y);
    return DAG.getNode(HandOpcode, DL, VT, Logic);                                   
  }
```

I noticed it just because I was referencing D6407 , It's minor and not directly related to the improvement this time. I'm not sure if this kind of change can be included.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149699