[PATCH] D25517: [SLPVectorizer] Improved support of partial tree vectorization.

[Hal Finkel via llvm-commits]

hfinkel added a comment.

Have you checked the compile-time impact of this change? I wonder if you need some additional cutoff because it looks like you might be increasing the worst-case complexity of the algorithm here.



================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4459
+class WeakVHWithLevel final : public CallbackVH {
+  /// Child level.
+  unsigned Level = 0;
----------------
The comment here should explain that the child level corresponds to the operand index of the instruction.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4461
+  unsigned Level = 0;
+  /// Is this instruction should be vectorized?
+  bool IsInitial = true;
----------------
  /// Is this the instruction that should be vectorized, or are we now processing children (i.e. operands of this instruction) for potential vectorization?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4499
 /// If it is legal to match a horizontal reduction feeding
 /// the phi node P with reduction operators BI, then check if it
 /// can be done.
----------------
The comment here is out of date.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4547
+        if (P) {
+          Inst = dyn_cast_or_null<Instruction>(BI->getOperand(0));
+          if (Inst == P)
----------------
dyn_cast_or_null -> dyn_cast (here and below). BI->getOperand(0) should not return null.


https://reviews.llvm.org/D25517