[PATCH] D21405: [PGO] IRPGO pre-cleanup pass changes

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 15 20:17:42 PDT 2016


On Wed, Jun 15, 2016 at 5:34 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:

>
> On Jun 15, 2016, at 4:38 PM, Xinliang David Li <davidxl at google.com> wrote:
>
>
> On Wed, Jun 15, 2016 at 4:11 PM, Mehdi Amini <mehdi.amini at apple.com>
> wrote:
>
>>
>>
>> Sent from my iPhone
>>
>> On Jun 15, 2016, at 3:16 PM, Xinliang David Li <davidxl at google.com>
>> wrote:
>>
>>
>>
>> On Wed, Jun 15, 2016 at 3:09 PM, Sean Silva <chisophugis at gmail.com>
>> wrote:
>>
>>> silvas added a comment.
>>>
>>> In http://reviews.llvm.org/D21405#459242, @mehdi_amini wrote:
>>>
>>> > I really don't like the fact that the PGO pipeline will be different
>>> from the non-PGO pipeline (other than what is required for
>>> instrumentations). I wonder what other people will think of that.
>>>
>>>
>>> This was one of my initial concerns too:
>>> http://lists.llvm.org/pipermail/llvm-dev/2015-August/089058.html
>>> Jake and I (actually mostly Jake) did some basic measurements related to
>>> this and did not find any significant difference.
>>>
>>> Rong also did some measurements related to this in the initial RFC
>>> thread: http://lists.llvm.org/pipermail/llvm-dev/2015-August/089425.html
>>>
>>> I agree though: generally speaking, we should avoid running too many
>>> passes before instrumentation precisely because they cause divergence
>>> between the PGO and non-PGO pipelines
>>
>>
>>
>> I agree that we should limit the difference to the minimal required, but
>> keeping two completely different compilation modes (e.g. PGO vs nonPGO, O3
>> vs O2 etc) pipeline identical should be a non-goal.
>>
>>
>>
>> Can you elaborate? I have a different intuition.
>>
>
> 1) Obviously O2 pipeline can be different from O1/O0.
> 2) Even between O2 and O3, different modes  have different design
> constraints due to differences in user expectations. For instance at O3 we
> can enable passes (e.g. loop transformations) that may be too expensive for
> O2
> 3) Passes require instrumentation certainly will have its instrumentation
> specific transformation pass  + dependent analysis pass + dependent
> pre/post clean up passes.  IR PGO falls into this category.   An existing
> example is memory sanitizer. If you compare msan + O2 pipeline vs O2
> pipeline, it has additional post-cleanup passes. See
> addMemorySantiizerPasses
>
>
> The fact that O2 and O3 (and O0...) are different is obvious, on the other
> hand the fact that PGO is not orthogonal isn't to me, especially since it
> was until now.
>
> I'm not saying that the improvements that we can get by specializing for
> PGO are not worth a difference, but that I see any new difference as
> something undesirable in general. This has a maintenance and validation
> cost. The existing pipelines (O2/O3, LTO) are already not well documented
> and validated, and here we're adding more.
>


yes, I don't see us in disagreement here actually.  Nothing comes for free,
so any change (e.g, diverging pipeline) needs to examined case by case to
make sure the benefit outweighs the cost.   When you say "any new
difference as something undesirable in general", I guess it means that in
general when the default optimization pipeline gets mature, we rarely see
pipeline changes able to make any visible performance differences, so it is
usually not justifiable -- which I tend to agree.  Here the benefit of this
change to PGO users is *huge* and it is not specific to any benchmarks but
is almost universally benefiting most of the programs. I also consider the
cost of using a few existing mature passes very low (compared with
introducing a new pass). It may shake up the latent bugs due to the change,
but that is a good thing, not burden.

Having said that, I agree we need do our diligence to prune the unnecessary
passes there.

thanks,

David

>
> --
> Mehdi
>
>
>
>
>
> thanks,
>
> David
>
>
>
>
>
>>
>> Thanks,
>>
>> Mehdi
>>
>>
>>
>> David
>>
>>
>>> . Thankfully Jake and I found that on our games only pre-inlining was
>>> needed (running other optimizations before instrumentation did not help
>>> significantly for our test cases beyond the benefit of just pre-inlining).
>>>
>>> (note: due to PR27299, a run of simplifycfg after inlining is needed.
>>> But this run of simplifycfg doesn't really affect the performance of the
>>> instrumented build at all)
>>>
>>>
>>> http://reviews.llvm.org/D21405
>>
>>
>
   -
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160615/14c1c2bc/attachment.html>


More information about the llvm-commits mailing list