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

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

In https://reviews.llvm.org/D27811#623755, @RKSimon wrote:

> 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?


I think it is a good idea :-). After all, the opcode can only be `ISD::VECTOR_SHUFFLE` in this method. So, that bit of information doesn't really need to be stored in any entries.



================
Comment at: lib/Target/X86/X86TargetTransformInfo.cpp:614
+                                              ISD::VECTOR_SHUFFLE, LT.second))
+        return Entry->Cost;
+
----------------
RKSimon wrote:
> 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.
Ah I see. That makes sense.


Repository:
  rL LLVM

https://reviews.llvm.org/D27811