[PATCH] D132261: [SLP]Do not reduce repeated values, use scalar red ops instead.

[Valeriy Dmitriev via Phabricator via llvm-commits]

vdmitrie added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:13100-13103
+    if (VL.empty()) {
+      unsigned Cnt =
+          SameValuesCounter.lookup(TrackedToOrig.find(VectorizedValue)->second);
+      switch (RdxKind) {
----------------
vdmitrie wrote:
> ABataev wrote:
> > vdmitrie wrote:
> > > okay :-) that works too
> > > 
> > > just a n.i.t. remark: you could reduce arguments to just these if the code was a dedicated method:
> > > (Value *VectorizedValue, IRBuilderBase &Builder,  ArrayRef<Value *> VL, unsigned Cnt)
> > > 
> > > 
> > > 
> > It is not possible, both SameValuesCounter and TrackedToOrig are used for each elements of VL in the second switch to build correct constant vector.
> I don't see that.
> Isn't the following is the only use of them in this part (under condition VL.empty())?
> 
> ` unsigned Cnt =
>            SameValuesCounter.lookup(TrackedToOrig.find(VectorizedValue)->second);
> `
> Aha, I even overlooked that you don't actually use VL elements in this part too.
> So here are the only essential arguments for it:
> (Value *VectorizedValue, IRBuilderBase &Builder, unsigned Cnt)
Ah, I missed that you said about second switch. I was thinking about first part of the method.
May be I wasn't clear enough.
I meant to have a separate method for the part which is currently under VL.empty().
Value *emitScaleForReusedOps(Value *VectorizedValue, IRBuilderBase &Builder, unsigned Scale) {

... here goes the code which is now under VL.empty() ...

}




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132261