[PATCH] D96754: [NewPM] Use stale divergence analysis with SimpleLoopUnswitch

[Sameer Sahasrabuddhe via Phabricator via llvm-commits]

sameerds added a comment.

In D96754#2567456 <https://reviews.llvm.org/D96754#2567456>, @sameerds wrote:

> That is not true. The flow in the legacy pass manager is more involved. The function LoopPass::preparePassManager() actually makes sure that if a loop transform T running inside a loop pass manager M invalidates analyses used by other passes in M, then T is split out into a separate loop pass manager M'. Thus every instance of loop pass manager is responsible for making sure that the used analyses are recomputed before starting any passes. The divergence analysis is not computed in the middle of loop unswitching like you seem to be suggesting here.
>
> But now that I looked at it more closely, this does invalidate my original assumption that the legacy PM is providing a stale analysis. In that sense, my patch should not be seen as reproducing existing behaviour ... it's turning out to be more of a hack because the new PM lacks some functionality. There doesn't seem to be a way for a transform in the new PM to isolate itself from the effects of other transforms that invalidate analyses in the outer analysis manager.

I can think of a static solution for this, which won't be as flexible as the old PM, but it kinda fits in the general scheme of things for the new PM.

1. Any loop pass that depends on an analysis other than the standard loop analyses should return a list in a static method.
2. Rather than adding passes to an LPM directly, an outer manager should add them through a proxy. This proxy can own more than one LPMs and has the ability to enqueue function analyses between these LPMs.
3. Whenever addPass() is called on this proxy, it should check the loop pass to see if it requires any non-standard analyses. If yes, it should start a new LPM and enqueue the required analyses before the new LPM

I am not sure if any existing class can serve as this proxy, or we need to add something new.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96754