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

Elena Demikhovsky via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 29 03:36:16 PST 2016


delena added a comment.

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

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


May I ask you to postpone refactoring of getShuffleCost() to the next commit? Andrea, I have another patch https://reviews.llvm.org/D28118 that also changes ShuffleCost for X86 targets.


Repository:
  rL LLVM

https://reviews.llvm.org/D27811





More information about the llvm-commits mailing list