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

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

In https://reviews.llvm.org/D27811#623731, @andreadb wrote:

> I noticed that `getShuffleCost` is becoming quite big.


You're not kidding ;-)

> Do you think it makes sense to split the logic in `getShuffleCost` in three parts (a function for each supported ShuffleKind)?. In case, `getShuffleCost` could be refactored in a separate commit.

I have considered hijacking the 'ISD::VECTOR_SHUFFLE' int entry in both CostTbleEmtry and CostTableLookup to do lookup based on TTI::ShuffleKind instead - what do you think?



================
Comment at: lib/Target/X86/X86TargetTransformInfo.cpp:614
+                                              ISD::VECTOR_SHUFFLE, LT.second))
+        return Entry->Cost;
+
----------------
andreadb wrote:
> Shouldn't this be `LT.first * Entry->Cost` ?
> I think that you are not accounting for the type legalization cost.
That's what I meant in the disclaimer at the top - as we're broadcasting we only reference the first input register and all the outputs are the same - so the costs aren't multiplied by the LT.first scale factor (num vectors). It doesn't account for any register moves that occur but that's true for most throughput costs.


Repository:
  rL LLVM

https://reviews.llvm.org/D27811