[PATCH] D27811: [CostModel][X86] Add support for broadcast shuffle costs

[Michael Kuperstein via Phabricator via llvm-commits]

mkuper added inline comments.


================
Comment at: lib/Analysis/CostModel.cpp:93
 
+static bool isBroadcastVectorMask(SmallVectorImpl<int> &Mask) {
+  for (unsigned i = 0, MaskSize = Mask.size(); i < MaskSize; ++i)
----------------
RKSimon wrote:
> mkuper wrote:
> > We already have this helper in CGP (that version also checks if you're splatting *any* element, not just element 0.)
> > Maybe make it common? Not entirely sure what the appropriate place for it yes, though. Would it make sense for CGP to use CostModel?
> Adding isSplatMask/isSplat/getSplatIndex support to ShuffleVectorInst (worth adding them to all to match ShuffleVectorSDNode ?) would be trivial, then both CodeGenPrepare and CostModel could use it easily.
> 
> It would probably be worth upgrading SK_Broadcast at the same time to support broadcasting any Index value (not just 0) - given that almost nothing uses it so far that shouldn't be a problem.
> 
> What other cost model cases did you have in mind? CodeGenPrepare::optimizeShuffleVectorInst seems quite limited.
> It would probably be worth upgrading SK_Broadcast at the same time to support broadcasting any Index value (not just 0) - given that almost nothing uses it so far that shouldn't be a problem.
Thinking about it a bit more, I'm not entirely sure about this. Ignoring what we do in the DAG for a moment, in IR, I'd expect the canonical insert + splat pattern to use index 0.

> What other cost model cases did you have in mind? CodeGenPrepare::optimizeShuffleVectorInst seems quite limited.
I didn't, really, just trying to avoid code duplication.



Repository:
  rL LLVM

https://reviews.llvm.org/D27811