[PATCH] D28961: [SLP] Fix for PR31690: Allow using of non-zero initial values in horizontal instructions.

Michael Kuperstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 20 18:31:43 PST 2017

mkuper added inline comments.

Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4256
+        // Check if the NextV argument can be used later after vectorization.
+        if ((EdgeToVist == 0 || TreeN->isAssociative()) &&
+            !ExtraArgs.count(TreeN)) {
I don't understand the logic here.
IIUC, if EdgeToVist is 0 it just means that we're currently going "left", but this could be at any stage in the reduction. Why does EdgeToVist == 0 means we can do this for a non-associative FAdd?

Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4329
+        if (auto *I = dyn_cast<Instruction>(V))
+          Builder.SetCurrentDebugLocation(I->getDebugLoc());
+        VectorizedTree = createBinOp(Builder, ReductionOpcode, VectorizedTree,
Don't you want the debugloc for the original op (Pair.first?), not for its operand?

Comment at: test/Transforms/SLPVectorizer/X86/horizontal-list.ll:748
   ret float %add.31
Could you please add a test that has several "extra" values?
It doesn't have to have so many vector lanes, that's just noise.


More information about the llvm-commits mailing list