[PATCH] D99719: [SLP] Better estimate cost of no-op extracts on target vectors.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:3535
+                Cost -= TTI->getVectorInstrCost(Instruction::ExtractElement,
+                                                VecTy, I);
+            }
----------------
fhahn wrote:
> ABataev wrote:
> > fhahn wrote:
> > > ABataev wrote:
> > > > I think you need to use the real extract indices here to be more correct, i.e.
> > > > ```
> > > > for (unsigned I = Idx - EltsPerVector; I <= Idx; ++I)
> > > > Cost -= TTI->getVectorInstrCost(Instruction::ExtractElement,
> > > >                                                 cast<ExtractElementInst>(VL[I])->getVectorOperandType(), *getExtractIndex(cast<Instruction>(VL[I])));
> > > > ```
> > > Thanks, I think that's much better, updated.
> > Hmm, I think more correct would be to do something like this:
> > ```
> > Cost += TTI->getShuffleCost(
> >                   ShuffleKind.getValue(),
> >                   FixedVectorType::get(VecTy->getElementType(), EltsPerVector), Mask);
> > ```
> > in case if `AllConsecutive` is `false` and ignore the initial shuffle cost completely. Also, you need to calculate the correct `Mask` here or pass `llvm::None`
> > 
> > So, I think you need to split it into separate parts.
> > The first one for `*ShuffleKind == TargetTransformInfo::SK_PermuteSingleSrc` with improved shuffle cost for non-consecutive extracts and the generic one with the old functionality
> That sounds good! I put up D99745. Was that what you had in mind for preparation? (Still need to add some comments, but I wanted to make sure that's what you actually had in mind)
Sorry, I did not mean to split the patch into 2 parts :) I meant to split processing into 2 parts. Plus, the patch cannot be NFC since it changes the functionality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99719