[PATCH] D31459: [Polly][NewPM] Port ScopDetection to the new PassManager

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

In https://reviews.llvm.org/D31459#713310, @philip.pfaffe wrote:

> In https://reviews.llvm.org/D31459#713119, @Meinersbur wrote:
>
> > Thanks for getting this for started. The part of adding those passes to the new pass manager's pipeline must be done in RegisterPasses.cpp, but all passes must be registered to the new pass manager, otherwise it cannot use them.
>
>
> Pass Registration is still a pending issue. Until https://reviews.llvm.org/D11032 is ready to land, there is no defined way to set up a polly pipeline.


I was trying to ask Chandler Carruth ask about this in the Hacker's Lab. Unfortunately I already asked too many questions and he preferred to let other people ask questions as well.

>> Note that we have two correctness issues that are fragile with the pass managers:
>> 
>> 1. Polly's codegen can change the IR of other SCoPs. Particularly it can make previously detected scops invalid. Thats why the sequence must be:
> 
> This is precisely the problem a ScopXManager would solve...

What is ScopXManager? If it is a single pass (Tobias suggested "-polly-sched") that would run all the other passes itself, then we have full control and can run in any order we like. However, we then also don't need to convert to existing passes to the new pass manager, they would be not be scheduled by any LLVM's pass manager anymore.

>> CodeGeneration of A can invalidate B, resulting in miscompiles because the IR of B was changed and does not correspond to its ScopInfo anymore.
> 
> ... however this violates fundamental assumptions both the new and(!) the old pass managers make. It's an upstream miscompile or crash waiting to happen.

This happens with Loop passes all the time. Loop passes can create new loops (loop distribution) or remove them as it they like, as long as the LoopInfo remains consistent.

> In what way do SCoPs specifically interact? If there is no way to break that dependence, we can't run our own SCoP pipeline and must widen the IRUnit of the analyses to Function.

I did not observe by myself (probably because it is already handled), but according to Tobias it happened in the past.

What I can think of is that the exit block of one SCoP is the entry block of the next one (and they are not combined to a single SCoP, e.g. because there is another edge from somewhere to that block). After codegen, there will be a merge block (`polly.merge_new_and_old`), combining the control flow from the the loop versioning branch, which would be the new entry block of the second. However, if ScopInfo already ran on the second SCoP, it will still reference the original entry block, which landed somewhere else, e.g. as the original code section.

>> 2. Polly does a bad job preserving analyses. It does not create new regions and loops in generated code. There is currently an ugly hack called NoopBarrier added to the pass pipeline that effectively throws away all analyses with the legacy pass manager. I get miscompiles when that barrier pass is removed. The new pass manager just caches all analyses.
> 
> This is a non-issue. The new PM pessimistically invalidates all cached analysis results after a transfrom by default.

Because pass dependencies are mostly transitive, invalidating all analysis would mean it also invalidates ScopDetection and we would try to re-detect the output of CodeGeneration. This is actually because polly passes have to preserve _all_ analyses used by any polly pass. It is also inefficient because we'd need to run all these analyses multiple times.


https://reviews.llvm.org/D31459