[PATCH] D115757: [SLP]Generalize cost model.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6313
+          ScalarCost = VL.size() *
+                       ScalarEltCost(std::distance(VL.begin(), find(VL, VL0)));
+        } else {
----------------
vdmitrie wrote:
> ABataev wrote:
> > RKSimon wrote:
> > > ABataev wrote:
> > > > RKSimon wrote:
> > > > > Is this purely compile time saving?
> > > > Replaced by 0, since it does not matter.
> > > I'm not sure if we need to pull them out into separate methods if they're just being called once. But it would help a little for readability if the lambdas weren't embedded in the GetCostDiff calls themselves.
> > > 
> > > So:
> > > ```
> > > auto ScalarEltCost = [](unsigned Idx)  {...};
> > > auto VectorCost = [](InstructionCost CommonCost)  {...};
> > > return GetCostDiff(ScalarEltCost, VectorCost);
> > > ```
> > I prefer this approach. I don't want to introduce virtual member functions. Without virtual functions the only approach - generic lambda, which increases codesize.
> What problem you see with virtual class here?
> Using named lambdas instead of unnamed ones is definitely better. But essentially it anyway remains a C-style programming,  just using C++ syntax. You are also making it very difficult to customize this code downstream. If that is the goal - okay, got it.
It usually increases compile time.
Why it is hard to customize it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115757/new/

https://reviews.llvm.org/D115757