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

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D110057#3048135 <https://reviews.llvm.org/D110057#3048135>, @nikic wrote:

> 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)
>
> Even if LoopFlatten is in its own LPM, doesn't it still break the LPM contract by not preserving analyses? Wouldn't the function to loop pass adaptor still claim the standard analyses are preserved, even though they actually aren't?
>
> I don't understand why this is not already triggering verification failures.

Ah, I think the reason is that it actually does preserve the analyses, or at least some subset of them: https://github.com/llvm/llvm-project/blob/d550930afcbb84740681c219ab13efd133143f88/llvm/lib/Transforms/Scalar/LoopFlatten.cpp#L645 https://github.com/llvm/llvm-project/blob/d550930afcbb84740681c219ab13efd133143f88/llvm/lib/Transforms/Scalar/LoopFlatten.cpp#L663-L665

Possibly it already preserves everything properly and just needs to report that it does? Plus report the deleted loop to the pass manager?


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

https://reviews.llvm.org/D110057