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

[Xinliang David Li via llvm-commits]

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.

> I guess they can’t be FunctionPass because they create global variables for the counters...

yes, it will create counters.

David


>
> Thanks,
>
> Mehdi
>