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

[Ta-Wei Tu via Phabricator via llvm-commits]

TaWeiTu added a comment.

In D87045#2274009 <https://reviews.llvm.org/D87045#2274009>, @Whitney wrote:

> In D87045#2265645 <https://reviews.llvm.org/D87045#2265645>, @TaWeiTu wrote:
>
>> In D87045#2264236 <https://reviews.llvm.org/D87045#2264236>, @ychen wrote:
>>
>>> - How about using `PreservedAnalyses run(Loop &, LoopAnalysisManager &, LoopStandardAnalysisResults &, LPMUpdater &, LoopNest &);` for the interface. I think it should work better because it is still a loop pass interface.
>>
>> Sounds like a good idea to me. Let' see what @Whitney and @etiotto think about the new interface first, and I'll update the patch later if they agree on that. Thanks!
>
> I am not strongly against it, however is not clear to me what the extra argument `Loop &` provide, as we can get the loop from `LoopNest &` by `LN.getOutermostLoop()`.

Thanks for your comment! 
I think the benefit of the new interface is that it shows that the loop-nest pass is just a special case of loop pass that provides an additional `LoopNest` object.
However, I think your concern about passing redundant parameters to the function is also valid.
Both interfaces seem equally good for me actually, and I don't have any preference for either of them.

I think maybe we can stick to the current interface for now (unless @ychen has a strong preference for the other one) and update it in the future if necessary.
Any thoughts?

By the way, is there any other concern regarding this patch other than the interface now?

Thanks again for your review and time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87045