[PATCH] D30103: [SLP] Rework `findBuildAggregate()` from ercursive form to iterative, NFC.

[Michael Kuperstein via Phabricator via llvm-commits]

mkuper accepted this revision.
mkuper added a comment.
This revision is now accepted and ready to land.

LGTM with minor comments.



================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4617
                                SmallVectorImpl<Value *> &BuildVectorOpds) {
-  if (!IV->hasOneUse())
-    return false;
-  Value *V = IV->getAggregateOperand();
-  if (!isa<UndefValue>(V)) {
-    InsertValueInst *I = dyn_cast<InsertValueInst>(V);
-    if (!I || !findBuildAggregate(I, BuildVector, BuildVectorOpds))
+  Value *V = IV;
+  do {
----------------
Any reason this isn't nullptr? It looks like you never use the initial value.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4628
+  } while (true);
+  std::reverse(BuildVector.begin(), BuildVector.end());
+  std::reverse(BuildVectorOpds.begin(), BuildVectorOpds.end());
----------------
Does the order really matter?
Anyway, if you think it does, I'm fine with this to keep it NFC.


https://reviews.llvm.org/D30103