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

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:13101-13119
+      // root = mul prev_root, <1, 1, n, 1>
+      if (VL.empty()) {
+        unsigned Cnt = SameValuesCounter.lookup(
+            TrackedToOrig.find(VectorizedValue)->second);
+        Value *Scale = ConstantInt::get(VectorizedValue->getType(), Cnt);
+        LLVM_DEBUG(dbgs() << "SLP: Add (to-mul) " << Cnt << "of "
+                          << VectorizedValue << ". (HorRdx)\n");
----------------
vdmitrie wrote:
> What I basically see here is that you packed two methods into one and differentiate them by VL argument.
> May be just split this method in two ?  One to handle identity optimization and another for same scale factor optimization. They seems do not share any essential code, only the switch. But sharing the switch does not look right to me as these optimizations do not fully share reduction kinds.
I thought about it, just in many cases we reuse the same code, so I decided to put it into single switch (just (f)add and xor have special processing).


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:13181
+                          << VectorizedValue << ". (HorRdx)\n");
+        return Builder.CreateFMul(VectorizedValue, Scale);
+      }
----------------
vdmitrie wrote:
> Are you relying on instcombiner to optimize this further? I mean for example mul X, 2 -> add X, X
Yes, because there might be some other numbers, say 3, 4, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132261