[PATCH] D154205: [MachineLICM] Handle subloops

[Dave Green via Phabricator via llvm-commits]

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

Thanks. LGTM



================
Comment at: llvm/lib/CodeGen/MachineLICM.cpp:135
     // Exit blocks for CurLoop.
     SmallVector<MachineBasicBlock *, 8> ExitBlocks;
 
----------------
jaykang10 wrote:
> dmgreen wrote:
> > jaykang10 wrote:
> > > dmgreen wrote:
> > > > jaykang10 wrote:
> > > > > dmgreen wrote:
> > > > > > Will ExitBlocks be incorrect now?
> > > > > Ah, that is good point!
> > > > > They are out-most loop's ExitBlocks.
> > > > > Let me fix it.
> > > > > Thanks for checking it.
> > > > > 
> > > > Could this use CurLoop->isLoopExiting(ExitBlocks) instead? It might be quicker for larger loops.
> > > This function checks exit blocks which are outside loop and have predecessor inside loop.
> > > isLoopExiting checks exiting blocks which are inside loop and have successor outside loop.
> > > I think we need exit blocks here.
> > > Let me try to keep the exit blocks for each loop in order to avoid re-calculation.
> > Oh I see. A different type of Exit Block.
> > Could it check `!CurLoop->contains(ExitBlocks) && any_of(ExitBlocks->predecessors, is in CurLoop)`?
> `CurLoop->getExitBlocks` collects blocks which are outside CurLoop and has predecessor in CurLoop.
> This function checks the MBB parameter is in the blocks.
Yeah - That was what I was aiming to capture. The ExitBlocks is outside the loop, but one of it's predecessors is inside.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154205/new/

https://reviews.llvm.org/D154205