[PATCH] D103246: [SelectionDAG] Extend FoldConstantVectorArithmetic to SPLAT_VECTOR

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:5301
+    return Op.isUndef() || Op.getOpcode() == ISD::CONDCODE ||
+           Op.getOpcode() == ISD::SPLAT_VECTOR || (BV && BV->isConstant());
   };
----------------
craig.topper wrote:
> Why don't we need to check that the splat vector operand is constant/undef like we do for build_vector?
Oh, good catch! I'm surprised nothing failed. I'll see to that.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:5371
 
-  SDValue V = getBuildVector(VT, DL, ScalarResults);
+  SDValue V = NumElts.isScalable() ? getSplatVector(VT, DL, ScalarResults[0])
+                                   : getBuildVector(VT, DL, ScalarResults);
----------------
craig.topper wrote:
> When I made similar changes to FoldConstantArithmetic, I was asked to create a splat vector if the input was splat vectors. Should we be consistent? Though maybe it's harder because you're dealing with more than 2 sources?
Yeah that was my thinking, we may be dealing with mixed splat_vector and build_vector here. Hexagon would certainly exercise that path. We can't get it perfect here. We could certainly add heuristics like:

if all `ScalarResults` are equal, and:

1. if any `Ops` was a splat
2. if the majority of `Ops` were splats
3. splat is legal for this data type
4. splat is legal and custom for this data type

.. then produce a splat. Or a combination thereof. I'm not sure what's preferable. We could assume that the target would canonicalize any BUILD_VECTOR into SPLAT_VECTOR but I don't know if we can guarantee that either.

I'm also concerned that Hexagon seems to be the only target which uses SPLAT_VECTOR for fixed-length vectors and we don't have great coverage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103246