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

Andrea Di Biagio via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 5 03:53:27 PST 2017


andreadb added inline comments.


================
Comment at: lib/Analysis/CostModel.cpp:520-522
+      if (Shuffle->isSplat(&BroadcastIndex) && BroadcastIndex == 0)
+        return TTI->getShuffleCost(TargetTransformInfo::SK_Broadcast,
+                                   VecTypOp0, 0, nullptr);
----------------
RKSimon wrote:
> andreadb wrote:
> > Is this code still needed?
> > r290810 introduced a check for `isZeroEltBroadcastVectorMask' at line 530.
> Thanks - I missed that for some reason.
> 
> What should we do? Keep with the separate Broadcast matching helpers or merge and use ShuffleVectorInst::isSplat ?
Good question. I don't have a strong opinion since both approaches sound reasonable to me.

In general, I like the idea of having separate helpers to check shuffle masks, and I am not particularly worried about the small code duplication in `isZeroEltBroadcastVectorMask` (since that helper is very simple).


Repository:
  rL LLVM

https://reviews.llvm.org/D27811





More information about the llvm-commits mailing list