[PATCH] [Refactor] Replace RegionPasses by FunctionPasses
Tobias Grosser
tobias at grosser.es
Wed Mar 4 07:22:22 PST 2015
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
More information about the llvm-commits
mailing list