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

[David Majnemer via Phabricator via llvm-commits]

majnemer added inline comments.


================
Comment at: lib/Transforms/Scalar/LICM.cpp:348
+  SmallVector<BasicBlock *, 16> Worklist;
+  DT->getDescendants(N->getBlock(), Worklist);
 
----------------
sanjoy wrote:
> majnemer wrote:
> > sanjoy wrote:
> > > `getDescendants` seems to be doing a BFS.  Given that (and that DT is a tree), can you iterate `Worklist` in reverse to get a post order?  If you do this, we'd have to spec `getDescendants` as doing a BFS though (and not just have it be an implementation detail).
> > > 
> > > OTOH if you write the BFS by hand (instead of relying on `getDescendents`), you can add an early exit for `CurLoop->contains(BB)`.
> > Do you mean BFS or DFS?
> I meant BFS; since DT is a tree you can do a (slightly) cheaper in-place BFS as:
> 
> ```
> Order.push_back(N);
> for (int i = 0; i < Order.size(); i++)
>   Order.push_back(N->children());
> ```
> 
I don't believe reverse iterating a BFS gives you a post-order.


https://reviews.llvm.org/D35609