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

[Adam Nemet via Phabricator via llvm-commits]

anemet added a comment.

Hi Alexey,

I've just come across this commit.  I don't think it is up to LLVM standards in terms of understandably.  Please improve; I've provided some questions inline.

Thanks,
Adam



================
Comment at: llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp:4457
+namespace {
+/// Tracks instructons and its children.
+class WeakVHWithLevel final : public CallbackVH {
----------------
Tracks for what?


================
Comment at: llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp:4499-4505
 /// \brief Attempt to reduce a horizontal reduction.
 /// 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 phi node P with reduction operators Root in a basic block BB, then check
+/// if it can be done.
 /// \returns true if a horizontal reduction was matched and reduced.
 /// \returns false if a horizontal reduction was not matched.
+static bool canBeVectorized(
----------------
The name of the function and the comment mismatch.  What is this function supposed to do?


================
Comment at: llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp:4517-4573
+  SmallVector<WeakVHWithLevel, 8> Stack(1, Root);
+  SmallSet<Value *, 8> VisitedInstrs;
+  bool Res = false;
+  while (!Stack.empty()) {
+    Value *V = Stack.back();
+    if (!V) {
+      Stack.pop_back();
----------------
This needs a description of the algorithm.


Repository:
  rL LLVM

https://reviews.llvm.org/D25517