[llvm-dev] loop transforms and function analyses; especially divergence analysis

[Sameer Sahasrabuddhe via llvm-dev]

---- On Fri, 29 Jan 2021 01:55:44 +0530 Arthur Eubanks <aeubanks at google.com> wrote ----

 > There is an option to enable SimpleLoopUnswitch instead of LoopUnswitch in the legacy PM, but I doubt anybody is using that, so I wouldn't worry about it.

Right. But SimpleLoopUnswitch does support the legacy PM, and the check for divergence is at an internal point independent of the pass manager. So either way, I have to make sure it plays well with both pass managers. But like you said, it's not super important, and I can choose to do something simple at that point.
 
 > Looking more closely at where divergence analysis is used, it looks like it's only used for non-trivial loop unswitching. In the short term, we could just disable non-trivial unswitching if we detect that the target has divergence. In fact, non-trivial unswitching was only turned on in the new PM recently at -O3.

Yeah, that's definitely something we want to consider. But it all depends on how important are the non-trivial cases in actual programs. We need to keep track of any potential performance regression from this decision, otherwise it will get wrongly attributed to the new pass manager. A simple way is to make the same change in LoopUnswitch with the legacy PM and see how it affects existing programs.

  > If we want to add something to LoopStandardAnalysisResults, I believe all loop passes must preserve everything in LoopStandardAnalysisResults, and updating all passes to handle divergence would be a pain. asbirlea should be able to provide more info.

That is clearly not desirable; divergence analysis is not used frequently enough to justify the effort.

 > An alternative is to skip the whole analysis infra and recalculate it on every run of SimpleLoopUnswitch. The LoopUnswitch pass doesn't preserve divergence analysis, so the analysis would be invalidated every time the pass successfully unswitches. If it doesn't unswitch very many loops right now, there could be a large compile time impact.

I assume you mean SimpleLoopUnswitch in your second sentence? Skipping the infra would undermine the purpose of having that infra in the first place. But even if we did skip it, divergence analysis will have to be recomputed every time SimpleLoopUnswitch is invoked, since it's all the other loop passes that will invalidate it. That would mean at least once per each loop, which sounds rather costly. That is why I think it would be better to have a second version that runs on the whole Function once. Then it's up to each specific compilation flow to decide how to order it relative to the loop passes.

Sameer.