[PATCH] [Refactor] Replace RegionPasses by FunctionPasses

[Tobias Grosser]

On 04.03.2015 15:33, Johannes Doerfert wrote:
> On 03/04, Tobias Grosser wrote:
>> On 04.03.2015 11:14, Johannes Doerfert wrote:
>>> Hey Tobias,
>>>
>>> I attached 2 lnt reports to this mail. The first (report_region.json)
>>> was created using the current polly/master. The second
>>> (report_function.json) was created with the region->function pass patch.
>>> Note that this patch is not yet at its full capacity, however over the 3
>>> runs I measured I got:
>>>
>>>    Performance Regressions  16
>>>    Performance Improvements  159
>>>    Unchanged Tests   817
>>>    Total Tests   992
>>>
>>> Where as the execution time changes (only 8 in total) should be jitter.
>>>
>>> The point I try to make here is, 3 runs with the region pass manager take
>>>
>>>    2:35:27 (3 runs region passes)
>>>
>>> but 3 runs with the function pass manager only
>>>
>>>    2:07:48 (3 runs function passes)
>>>
>>> hence we save ~18% of the lnt time (if I didn't do a stupid math mistake).
>>>
>>> To be precise, we save compile time "only" for larger functions with small SCoP
>>> coverage but we do not pay for it in any other case.
>>
>> Amazing. The performance results are really interesting.
>>
>>> Does this convince you that region passes have a bad impact on compile time?
>>
>> Sure.
>>
>> (Even before I was convinced going somewhere in this direction is a good
>> idea, but I wanted to understand the reasons/motivations for the changes as
>> I had the feeling there are several good reasons to go into this direction).
>>
>> What are the next steps you would like to take? Is this the patch you would
>> like me to review or are you planning to split/change something beforehand?
> See below.
>
>> I currently only commented on the DependenceInfo changes, but I have the
>> feeling they could possibly be split out nicely without yet having any
>> performance impact. What do you think?
> I wanted (positive) initial feedback first before I put more work into this.

I like the idea of dropping region passes, but I do not yet fully 
understand the effects of the region to function pass change (especially 
in the light of Chandler's pass manager changes).

> I will split the patch and commit it in 2-5 parts that are will be more
> or less small and self contained. However, the actual change from Region
> to Function pass in ScopPass will be a bit more complicated.

That would simplify reviewing the changes a lot.

> P.S.
>    What's your opinion on the renaming of Dependences to DependenceInfo?

I do not really have a strong opinion on the rename itself, but I 
believe this should be done independently. It is a large change that 
adds a lot of noise and it seems to be mostly independent of the region 
to function pass change.

To my understanding, the rename was mostly motivated by you introducing 
the Dependences class, that keeps information about dependences. This by 
itself seems a great idea and it also fits very much the pass-manager 
design Chandler has in mind. The only change needed is to
make the dependences object should also contain all dependence analysis 
methods (besides the actual pass infrastructure). Like this, you do not 
have the D.* sprinkled all over the place. (See the LoopAnalysis as an 
example).

Cheers,
Tobias