[PATCH] D87045: [LoopNest] Handle loop-nest passes in LoopPassManager

[Yuanfang Chen via Phabricator via llvm-commits]

ychen accepted this revision.
ychen added a comment.

In D87045#2302107 <https://reviews.llvm.org/D87045#2302107>, @TaWeiTu wrote:

> In D87045#2301614 <https://reviews.llvm.org/D87045#2301614>, @ychen wrote:
>
>> From http://llvm.org/OpenProjects.html#llvm_loopnest, it was mentioned that
>> "If you decide to write it as a loop pass, then you are wasting compile time to traverse to your pass and return right away when the given loop is not the outermost loop."
>>
>> Taking another look at the overall approach of this patch: currently for all loops (top-level or nested) in a function, when encountering a loop nest pass in LPM, a LoopNest is calculated and the loop nest pass is called with the computed LoopNest. Does this help compile-time in practice? It feels like it is essentially a loop pass that got passed a LoopNest as an incoming parameter that could be equally implemented (without this patch) with a normal loop pass that computes LoopNest by itself? This seems does not solve the problem quoted  above.
>>
>> To achieve that, I think we need to change FunctionToLoopPassAdaptor to dispatch top-level loops (aside from to regular loops and LPMs) to a LoopNestPassManager managing only loop nest passes or a loop nest pass directly.
>
> If I understood correctly, I think the follow-up patch D87531 <https://reviews.llvm.org/D87531> is designed to address the issue.

ha, thanks. I missed this one.

> Specifically, in `FunctionToLoopPassAdaptor`, if the input pass is a loop pass or an LPM containing both loop passes and loop-nest passes, all loops in the function have to be processed regardless if they're top-level or not.
> On the other hand, if the input pass is a loop-nest pass or is an LPM containing only loop-nest passes, only top-level loops are sent to the worklist and processed.

Could you please add comments about this either before LPM or before `FunctionToLoopPassAdaptor`? It is helpful for understanding the code.

> Does this solve the problem you mentioned? Thanks!

Yes, totally.

LGTM.  Please hold off a bit in case @asbirlea wants to chime in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87045