[PATCH] D96345: [DAG] Fold shuffle(bop(shuffle(x,y),shuffle(z,w)),bop(shuffle(a,b),shuffle(c,d)))

[Pengfei Wang via Phabricator via llvm-commits]

pengfei accepted this revision.
pengfei added a comment.
This revision is now accepted and ready to land.

I didn't verify the generated binary, but the math LGTM.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:21072
+      SDValue Op11 = N1.getOperand(1);
+      if (Op00.getValueType() == VT && Op10.getValueType() == VT &&
+          Op01.getValueType() == VT && Op11.getValueType() == VT &&
----------------
It seems all BinOps have the same VT between inputs and output. Nevertheless, I think we could still be able to do the merge even if they have different VTs.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:21083
+        SmallVector<int, 4> LeftMask;
+        if (Op00.getOpcode() == ISD::VECTOR_SHUFFLE &&
+            N0->isOnlyUserOf(Op00.getNode()) &&
----------------
Nit, can we use a lambda expression like below to simplify the 4 checks
```
auto canMergeInnerShuffle = [&](bool LeftOp, bool Commute) {
  SDValue Op0 = LeftOp ? Op00 : Op01;
  SDValue Op1 = LeftOp ? Op10 : Op11;
  SDValue &SV0 = LeftOp ? LeftSV0 : RightSV0;
  SDValue &SV1 = LeftOp ? LeftSV1 : RightSV1;
  SmallVector<int, 4> &Mask = LeftOp ? LeftMask : RightMask;
  if (Commute)
    std::swap(Op0, Op1);
  return Op0.getOpcode() == ISD::VECTOR_SHUFFLE &&
         N0->isOnlyUserOf(Op0.getNode()) &&
         MergeInnerShuffle(Commute, SVN, cast<ShuffleVectorSDNode>(Op0), Op1,
                           TLI, SV0, SV1, Mask) &&
         llvm::none_of(Mask, [](int M) { return M < 0; });
};
if (canMergeInnerShuffle(true, false) || canMergeInnerShuffle(true, true)) {
  LeftMerged = true;
} else {
  LeftMask.assign(SVN->getMask().begin(), SVN->getMask().end());
  LeftSV0 = Op00, LeftSV1 = Op10;
}
...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96345