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

Anna Thomas via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 30 07:52:25 PDT 2017


anna added a comment.

@anemet: We saw the degradations with respect to this change (https://reviews.llvm.org/D33320) itself, not before the original change. Here's the code change as I see it. Please correct if this is wrong:

1. we were initially processing exactly one node in `tryToVectorizeHorReductionOrInstOperands` equivalent function, before any changes below.
2. after https://reviews.llvm.org/D25517, we are processing at most 2 ^ 12 nodes in tryToVectorizeHorReductionOrInstOperands, but they are limited to a single basic block.
3. in this refinement change (https://reviews.llvm.org/D33320), we are processing at most 2 ^ 12 nodes, but they are no longer limited to a single basic block.

@ABataev is fixing #3 by limiting it to the same basic block so that it's actually an NFC wrt #2. That might fix the compile time regressions. However, my concern is that aren't we still having the possibility of high compile time impact in #2 when we have a single large basic block with 2^10 binary instructions for example (because we are not limiting the number of nodes, but rather the depth of the tree)? 
Should we perhaps have a threshold cutoff for the number of stack nodes? Or reduce the `MaxDepthRecursion` from 12 to 6?

I bailed out of this loop (return false in tryToVectorizeHorReductionOrInstOperands) when the stack size is too large, and that fixed our compile time regression temporarily.


Repository:
  rL LLVM

https://reviews.llvm.org/D33320





More information about the llvm-commits mailing list