[PATCH] D110057: [LoopFlatten] Move it to a LoopPassManager

Sjoerd Meijer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 21 00:28:56 PDT 2021


SjoerdMeijer added a comment.

In D110057#3010889 <https://reviews.llvm.org/D110057#3010889>, @asbirlea wrote:

> In D109958 <https://reviews.llvm.org/D109958> I missed the fact that LoopFlatten does not preserve any analyses. This is, I assume, the reason it was not added alongside other Loop passes before and probably also why I'm seeing a crash when adding it as part of LPM2.
> The pass should not misbehave on its own in a LoopPassManager because it's not using the Updater to readd loops for reprocessing, otherwise it would be incorrect alone too.
> So either LoopFlatten needs to be remain in it's own LPM or taught to preserve analyses (LoopInfo, DominatorTree, etc)

I think there are 2 separate things going on here:

- A recent patch is causing a problem (as reported in D109958 <https://reviews.llvm.org/D109958>), which I am investigating and fixing. I suspect this is the same crash that you're seeing.
- In addition, we have the question of preserved analysis. LoopFlatten removes an inner-loop, so it sounds right to me that LoopFlatten doesn't preserve LoopInfo, DomTree, or readd loops, etc. I was assuming that when a pass doesn't preserve an analysis, the pass manager reruns it when a subsequent pass requires it. But please advise if LoopFlatten is fine where it is here in this patch, or if it needs moving to its own LPM.


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

https://reviews.llvm.org/D110057



More information about the llvm-commits mailing list