[PATCH] D33320: [SLP] Improve comments and naming of functions/variables/members, NFC.

[Anna Thomas via Phabricator via llvm-commits]

anna added a comment.

In https://reviews.llvm.org/D33320#796679, @ABataev wrote:

> In https://reviews.llvm.org/D33320#795834, @anna wrote:
>
> > Hi Alexey, Adam,
> >
> > From what I can see in this algorithm, there is no limit on the actual size of the stack in the loop. The level variable controls just the recursion limit. So, in effect, IIUC, the max total number of operands being processed by the while loop is 2 ^ RecursionLimit (it's to the base 2 because we avoid phi nodes).
>
>
> It does not limits the number of processed nodes, it limits the tree height just like it was before.


Yes, but limiting the tree height itself is not enough right? Now, in the worst case,  2^12 nodes being processed in the `tryToVectorizeHorReductionOrInstOperands`, when earlier it was just a single node (i.e. before this change: https://reviews.llvm.org/D25517).



================
Comment at: llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp:4828
+    // Try to vectorize operands.
+    if (++Level < RecursionMaxDepth)
+      for (auto *Op : Inst->operand_values())
----------------
ABataev wrote:
> anna wrote:
> > Could you please explain how this is NFC wrt the previous code?
> > In the code on the LHS, we are checking that the operand is in the same basic block as the root before placing it on the stack.
> > 
> > Here we are unconditionally placing all operands on the stack.
> Yes, missed these checks, will add them
Yeah, we saw huge compile time degradations in tryToVectorizeHorReductionOrInstOperands. 


Repository:
  rL LLVM

https://reviews.llvm.org/D33320