[PATCH] D49225: [SLPVectorizer] Move scalar/vector costs to helper functions (NFCI).

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

In https://reviews.llvm.org/D49225#1165016, @RKSimon wrote:

> In https://reviews.llvm.org/D49225#1164986, @ABataev wrote:
>
> > In https://reviews.llvm.org/D49225#1164807, @RKSimon wrote:
> >
> > > I think the use of TTI->getInstructionCost in getScalarCost would be quite straightforward - it is getVectorCost that will need most of the custom handling as we're manipulating scalar Instructions to query equivalent vector costs.
> > >
> > > IMO we're better off using the getScalarCost/getVectorCost abstractions instead of embedding TTI->getInstructionCost calls directly inside BoUpSLP::getEntryCost - it notably helps simplify the code and improves readability.
> >
> >
> > I tried to do something similar some time ago, but I did not like it. Instead of one switch for opcode we have 2. But we already know the opcode in many cases and, actually, the second switch is required only for the shuffles. Maybe it is worth it to outline several standalone functions for PHIs, CmpInsts, BinOps etc. and use them directly where we know the opcode and use these getScalarCost/getVectorCost only for shuffles? Of course, these 2 functions also should call these outlined cost functions for each particular opcode.
>
>
> I can investigate ways to return std::pair<int, int> if you think that might be better? That would avoid the scalar/vector switch statement duplication, TTI->getInstructionCost will be doing something very similar.


Why do we need to return pair? You want to reuse the same function for scalar/vector? I meant, that we have the first switch when we looking through the opcodes in getEntryCost and then we have 2 additional switches when we calling getScalarCost and getVectorCost.
BTW, do we really need the separate cost for scalar/vector ops or we can calculate the final cost immediately? I mean, instead of getSclarCost/getVectorCost we may have just getPHICost, getBinOpCOst, etc. + getCost that will call all these functions for the shuffle? These functions will calculate the difference between the vector/scalar cost.

> An alternative would be to handle shufflevector/alternate separately (like we do for gather stages) and split getEntryCost into 2 functions - the special cases (gathers/alternates etc.) and the costs for particular opcodes.

Yes, probably it would be better and that's what I actually suggested.


Repository:
  rL LLVM

https://reviews.llvm.org/D49225