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

[Valeriy Dmitriev via Phabricator via llvm-commits]

vdmitrie 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);
----------------
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.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132590