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

[Adam Nemet via Phabricator via llvm-commits]

anemet added a comment.

Thanks for improving this!



================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4772
+  /// Must the instruction itself be vectorized?
+  bool instructionAnalysis() const { return !OperandIndexAnalyzed.hasValue(); }
   /// Try to vectorize children.
----------------
First word needs to be a verb, how about analyzingInstruction.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4774
   /// Try to vectorize children.
-  void clearInitial() { IsInitial = false; }
+  void analyzeOperands() { OperandIndexAnalyzed = 0; }
   /// Are all children processed already?
----------------
setAnalyzingOperands


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4776
   /// Are all children processed already?
-  bool isFinal() const {
-    assert(getValPtr() &&
+  bool operandsAnalyzed() const {
+    assert(getValPtr() && !instructionAnalysis() &&
----------------
areOperandsAnalyzed


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4785
   /// Get next child operation.
   Value *nextOperand() {
+    assert(getValPtr() && isa<Instruction>(getValPtr()) && !instructionAnalysis() &&
----------------
getNextOperand


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4820-4827
+  // Start analysis starting from Root instruction. If horizontal reduction is
+  // found, try to vectorize it. If it is not a horizontal reduction or
+  // vectorization is not possible or not effective, and currently analyzed
+  // instruction is a binary operation, try to vectorize the operands. If the
+  // operands were not vectorized, repeat the same procedure considering each
+  // operand as a possible root of the horizontal reduction.
+  // Interrupt the process if the Root instruction itself were vectorized or all
----------------
Please mention the actual traversal used.  It seems this is preorder.  If that is true why do you even need to track the operand index?  Iterative pre-order is:


```
while (!Stack.empty()) {
  Value *V = Stack.back();
  Stack.pop_back();
  visit(V);
  for (auto *O = V->rbegin(); O != V->rend(); ++O)
    Stack.push_back(*O);
}
```


https://reviews.llvm.org/D33320