[PATCH] D23410: [SLP] Initialize VectorizedValue when gathering

[Matthew Simpson via llvm-commits]

mssimpso added inline comments.

================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2175
@@ +2174,3 @@
+  auto ForwardStart = BasicBlock::iterator(Leader);
+  for (auto &I : make_range(ForwardStart, BB->end())) {
+    if (Bundle.erase(&I))
----------------
mkuper wrote:
> Just to make sure I understand this correctly - you're running from Leader, inclusive, so that if VL[0] is the last instruction in program order, you'll end up with LastInst == Leader, right?
> If so, it's probably worth a comment - or just initialize LastInst to Leader instead of null.
> 
> Also, won't we get worst-case quardatic behavior here, if VL[0] is consistently last (so we end up running to the end of the block for each bundle)?
Right, if VL[0] is consistently last, we'll end up running to the end of the block a lot. I think the common case here should be that VL[0] is first in program order? (But I could be wrong about this). Is there a better way to do this? setInsertPointAfterBundle doesn't really work as expected.

For this particular bug, another way to fix the insertion issue might be to modify Gather to change the insert point before generating each insertlement instruction. What do you think?




https://reviews.llvm.org/D23410