[PATCH] D109762: [NewPM][SimpleLoopUnswitch] Add DivergenceInfo

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added a comment.

In D109762#3070676 <https://reviews.llvm.org/D109762#3070676>, @aeubanks wrote:

> In D109762#3069261 <https://reviews.llvm.org/D109762#3069261>, @sameerds wrote:
>
>> In D109762#3059988 <https://reviews.llvm.org/D109762#3059988>, @aeubanks wrote:
>>
>>> I talked to some people and we've decided that the best thing to do would be to refactor out the nontrivial unswitching part into a function pass. Nontrivial unswitching is fairly special in the kinds of transforms it does.
>>
>> Will that always work as expected? The real dependency is that the divergence analysis is not incrementally updated. So even if this is a function pass, we may want to rerun it on the whole function every time it manages to unswitch a loop.
>
> With this patch, we have to rerun DivergenceAnalysis every time we run the pass on every loop. If we change nontrivial unswitching into a function pass, we can upgrade that to only having to be rerun every time we actually unswitch something.
> To be more specific, with the function pass, we'd have to invalidate most analyses anyway after every successful unswitch.
>
>   for every loop
>     DA = FAM.getAnalysis<DivergenceAnalysis>(); // this will not rerun the analysis if we haven't invalidated below because we didn't successfully unswitch
>     if successful unswitch
>       auto PA = PreservedAnalyases::none();
>       PA.preserve<SCEV>();
>       // and all the other existing analyses we preserved
>       FAM.invalidate(PA);

There's also the option to update all analysis inside the new function pass that's doing non-trivial unswitching. This is currently the case with all other analyses (they are updated), so the only one that needs to be added is DA. 
This limits the need to update DA to just the new Function pass, vs all Loop passes that are part of the same LPM as SimpleLoopUnswitch.
The alternative is, as you both already said, for DA to be recomputed when it is invalidated.


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

https://reviews.llvm.org/D109762