[PATCH] D132590: [SLP] Try to match reductions first in a vector build sequence.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:11820-11823
+  SmallVector<WeakTrackingVH> PostponedInsts;
+  for (Value *Op : BuildVectorOpds)
+    OpsChanged |=
+        vectorizeHorReduction(nullptr, Op, BB, R, TTI, PostponedInsts);
----------------
vdmitrie wrote:
> ABataev wrote:
> > vdmitrie wrote:
> > > ABataev wrote:
> > > > vdmitrie wrote:
> > > > > ABataev wrote:
> > > > > > Why do you want still call this after `findBuildAggregate`?
> > > > > In order to match hor reduction on build vector operands. Can you offer other approach to do the same?
> > > > But vectorizeRootInstruction already does this, why do you want to do it twice, here and in vectorizeRootInstruction? It increases compile time.
> > > > Can we just call vectorizeRootInstruction(nullptr, I, BB, R, TTI); and vectorizeBuildVector after? And remove this loop and final OpsChanged |= tryToVectorize(PostponedInsts, R); call?
> > > > But vectorizeRootInstruction already does this, 
> > > 
> > > Nope. This is exactly what does not happen after your commit https://reviews.llvm.org/rG7ea03f0b4e4e
> > > Before the commit we traversed through operands of insertelements  when collected instructions for work stack and were able to locate all reductions at once.
> > > 
> > > > Can we just call vectorizeRootInstruction(nullptr, I, BB, R, TTI); and vectorizeBuildVector after?
> > > 
> > > Nope. vectorizeRootInstruction may too early create 2-way vectorizations. We only want to call it after trying on with wider VFs with tryToVectorizeList.
> > > 
> > > 
> > Then you don't need to call vectorizeRootInstruction. Also, why do you need to find operands of the buildvector sequence? Just call vectorizeHorReduction for each insert instruction in the loop and in the second loop call findBuildAggregate and all other stuff for buildvector and postponed insts.
> > Just call vectorizeHorReduction for each insert ...
> 
> I don't like this approach. I'd rather enable vectorizeHorReduction to traverse through insertelement operands. BTW you did not explain why you suppressed that.
> 
To save compile time, to avoid analysis of the same instructions several times.
Why you don't like it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132590