[PATCH] D35609: [LICM] Make sinkRegion and hoistRegion non-recursive

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy accepted this revision.
sanjoy added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lib/Transforms/Scalar/LICM.cpp:349
+    for (DomTreeNode *Child : DTN->getChildren())
+      add_region_to_worklist(Child);
+  }
----------------
davide wrote:
> sanjoy wrote:
> > Not sure that the lambda is actually useful here -- why not just:
> > 
> > ```
> > if (CurLoop->contains(Child->getBlock()))
> >   Worklist.push_back(CurLoop);
> > ```
> > ?
> > 
> > If you wanted to go a bit further, I'd say even
> > 
> > ```
> > for (size_t i = 0; i < Worklist.size(); i++)
> >   copy_if(Worklist[i]->getChildren(), <lambda to check if node is in loop>, std::back_inserter(Worklist));
> > ```
> > 
> > may work.
> I personally find the lambda version slightly more readable.
In that case SGTM, but please use camel case. :)


https://reviews.llvm.org/D35609