[PATCH] D133441: [SLP] Look ahead for mutual horizontal reductions.

[Alexandros Lamprineas via Phabricator via llvm-commits]

labrinea added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:11035-11038
+    for (ArrayRef<Value *> RdxOps : ReductionOps)
+      for (Value *RdxOp : RdxOps)
+        if (RdxOp)
+          V.addHorizontalReduction(RdxOp);
----------------
ABataev wrote:
> labrinea wrote:
> > ABataev wrote:
> > > labrinea wrote:
> > > > ABataev wrote:
> > > > > Not sure it is a good idea to store reduction ops here, you're working with reduced values actually.
> > > > If you look at the reproducer:
> > > > 
> > > > ```
> > > > for i = ...
> > > >   sm += x[i];
> > > >   sq += x[i] * x[i];
> > > > ```
> > > > the addition `sm += x[i]` (which is a reduction op of the first horizontal reduction) is an external user of the scalar load from the multiplication `x[i] * x[i]` (which is a reduction value of the second horizontal reduction)
> > > Aha, this is for external users. Then why do you need to store there reduced values?
> > Because similarly the multiplication (reduced value) is an external user of the scalar load from the other horizontal reduction.
> Ah, I see what do you mean. But this is not the best place for gathering the reduced values, they must be gathered during an attempt to build the tree, since they might be not vectorized but gathered. If they are gathered, you cannot treat them as a future vector.
I see. In other words we cannot speculate that the vectorization will eventually happen before trying to actually reduce, so the look ahead approach I am suggesting is rather unsafe? By the time we are attempting to build the tree it is already too late.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133441