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

[Matthew Simpson via llvm-commits]

mssimpso added inline comments.

================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2178
@@ +2177,3 @@
+
+  // If BB hasn't been scheduled, we will need to find the last instruction by
+  // brute force. We iterate forwards from front (inclusive) until we either
----------------
mkuper wrote:
> mkuper wrote:
> > Could you please change this to be something like
> > 
> > ```
> > } else {
> >  // If BB hasn't...
> > ...
> > }
> > ```
> > ?
> > 
> > The dangling else really bugs me. :-)
> Also, how can it happen that the BB hasn't been scheduled?
> I thought this only gets called from within vectorizeTree(), which calls scheduleBlock() first?
No problem!

================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2178
@@ +2177,3 @@
+
+  // If BB hasn't been scheduled, we will need to find the last instruction by
+  // brute force. We iterate forwards from front (inclusive) until we either
----------------
mssimpso wrote:
> mkuper wrote:
> > mkuper wrote:
> > > Could you please change this to be something like
> > > 
> > > ```
> > > } else {
> > >  // If BB hasn't...
> > > ...
> > > }
> > > ```
> > > ?
> > > 
> > > The dangling else really bugs me. :-)
> > Also, how can it happen that the BB hasn't been scheduled?
> > I thought this only gets called from within vectorizeTree(), which calls scheduleBlock() first?
> No problem!
That's a good question... For the test case, an entry for BB is never created in BlocksSchedules because buildTree_rec exits early (max recursion depth). The BlocksSchedules entry isn't created until later on in the function, right before the "practice" scheduling. In vectorizeTree, the "real" scheduling only happens for the blocks in BlocksSchedules.

I bet if we move the BlockSchedules entry creation to the top of buildTree_rec, we can eliminate the "else" block here. I'll update the patch. Thanks!


https://reviews.llvm.org/D23410