[PATCH] D132412: [InstCombine] ease use constraint in tryFactorization()

[chenglin.bi via Phabricator via llvm-commits]

bcl5980 added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/add.ll:1759
 
+; x * y + x --> (y + 1) * x
+
----------------
arsenm wrote:
> arsenm wrote:
> > Do we have a backend combine to undo this already?
> Specifically, this is bad for us because it breaks integer mad matching 
There is a similar pattern in DAGCombiner but the `mul` second op is also constant:
https://github.com/llvm/llvm-project/blob/main/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp#L4112
```
  // fold (mul (add x, c1), c2) -> (add (mul x, c2), c1*c2)
  if (DAG.isConstantIntBuildVectorOrConstantInt(N1) &&
      N0.getOpcode() == ISD::ADD &&
      DAG.isConstantIntBuildVectorOrConstantInt(N0.getOperand(1)) &&
      isMulAddWithConstProfitable(N, N0, N1))
    return DAG.getNode(
        ISD::ADD, DL, VT,
        DAG.getNode(ISD::MUL, SDLoc(N0), VT, N0.getOperand(0), N1),
        DAG.getNode(ISD::MUL, SDLoc(N1), VT, N0.getOperand(1), N1));
```
And it looks AArch64 backend already has the pattern:
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp#L15066

```
  // Canonicalize X*(Y+1) -> X*Y+X and (X+1)*Y -> X*Y+Y,
  // and in MachineCombiner pass, add+mul will be combined into madd.
  // Similarly, X*(1-Y) -> X - X*Y and (1-Y)*X -> X - Y*X.
```
AMDGPU can add similar code, maybe you need to consider more for that (divergence? data type?).
https://alive2.llvm.org/ce/z/gJRkR5


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132412