[PATCH] D28118: AVX-512 cost calculation for interleave load/store patterns

[Simon Pilgrim via Phabricator via llvm-commits]

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

LGTM, with a few minors. I can do the NFC refactor on the shuffle kinds after that and the finish off the broadcast patch.

I wonder whether we should consider adding a version of getShuffleCost that takes a raw shuffle mask instead of a shuffle kind enum - similar to getIntrinsicInstrCost. There is always going to be cases where the target code can identify more cheap shuffle cases. But that is a problem for another day - we would still need the SK_PermuteTwoSrc/SK_PermuteSingleSrc enums.



================
Comment at: ../lib/Analysis/CostModel.cpp:115
+
+static bool isBroadcastVectorMask(SmallVectorImpl<int> &Mask) {
+  int SplatVal = -1;
----------------
mkuper wrote:
> delena wrote:
> > RKSimon wrote:
> > > I've added a ShuffleVectorInst::isSplat helper in D27811 for the same purpose. It's probably worth you merging that part (inc the CodeGenPrepare.cpp change as well) here.
> > The "broadcast" was not the matter of my patch and I do not really investigated in it, just added for completeness. And the broadcast-test is not full. I'd rather remove these changes at all and let you to proceed. 
> So, if this goes in first, it'll get removed by Simon's clean-up, right?
Yes, but that's not a problem.


================
Comment at: ../lib/Target/X86/X86TargetTransformInfo.cpp:617
       { ISD::VECTOR_SHUFFLE, MVT::v32i16, 1 }, // vpermw
+      { ISD::VECTOR_SHUFFLE, MVT::v16i16, 1 }, // vpermw
       { ISD::VECTOR_SHUFFLE, MVT::v64i8,  6 }  // vextracti64x4 + 2*vperm2i128
----------------
I missed this when I did the earlier work on Reverse - better just to commit this (and the fixed cost test) separately.


Repository:
  rL LLVM

https://reviews.llvm.org/D28118