[PATCH] D15828: [PGO] Passmanagerbuilder change that enable IR level PGO instrumentation

[Mehdi Amini via llvm-commits]

> On Jan 4, 2016, at 2:01 PM, Xinliang David Li <davidxl at google.com> wrote:
> 
> On Mon, Jan 4, 2016 at 1:55 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:
>> 
>>> On Jan 3, 2016, at 5:23 PM, David Li <davidxl at google.com> wrote:
>>> 
>>> davidxl added inline comments.
>>> 
>>> ================
>>> Comment at: lib/Transforms/IPO/PassManagerBuilder.cpp:281
>>> @@ -239,1 +280,3 @@
>>> 
>>> +  addPGOInstrPasses(MPM);
>>> +
>>> ----------------
>>> chandlerc wrote:
>>>> davidxl wrote:
>>>>> joker.eph wrote:
>>>>>> There will be an inliner SCC pass manager created in the PGO InstrPasses. It is not clear to me why you don't integrate this with the regular inliner a few line below.
>>>>> This is the pre-inline pass that needs to happen before instrumentation. See the original RFC: http://lists.llvm.org/pipermail/llvm-dev/2015-August/089425.html for details.
>>>> Yes, and there was not consensus in the RFC for the pre-inlining. Instead, the approach suggested was to work on independent components which seem non-controversial and have a separate focused discussion about pre-inlining and other interactions.
>>>> 
>>>> I think there are still a lot of unanswered questions around the pre-inlining phase (based on my reading of the RFC thread you pointed at). If there is some other thread that I've not seen, feel free to direct there...
>>> The original RFC discussion is here: https://groups.google.com/forum/#!topic/llvm-dev/ncONLhVJ3L0
>> 
>> Do you prefer the discussion to continue on the RFC thread or here?
> 
> Either way is fine.
>> 
>> I missed the fact that PGOInstrumentationGen/PGOInstrumentationUse are Module pass, I thought they were function passes and thus would be ran during the SCC Pass Manager with all the passes that follow the Inliner.
> 
> That will defeat the the purpose of PGO -- it needs to happen before
> the regular inline pass so that the inliner can be profile guided. The
> pre-inline is a dummy size reduction inline pass which does not need
> profile guidance.

Ok, I see, you want “light inline" -> apply profile data -> “full inline driven by profile data”

It “could”  still be done (not saying it necessarily should) with a single SCC pass over the module by having the “instrument” phase implemented as a SCC pass (which is allowed to create globals).

— 
Mehdi




> 
>> I guess they can’t be FunctionPass because they create global variables for the counters...
> 
> yes, it will create counters.
> 
> David
> 
> 
>> 
>> Thanks,
>> 
>> Mehdi