[PATCH] D33540: [PM/ThinLTO] Port the ThinLTO pipeline (both components) to the new PM.

[Teresa Johnson via llvm-commits]

On Tue, Jun 27, 2017 at 3:30 PM, Chandler Carruth <chandlerc at gmail.com>
wrote:

> On Tue, Jun 27, 2017 at 3:29 PM Teresa Johnson via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> On Tue, Jun 27, 2017 at 3:19 PM, Tim Shen <timshen at google.com> wrote:
>>
>>> Clang sets lto::Config::UseNewPM. I suppose at somewheter in
>>> lib/LTO/LTOBackend.cpp we should look at UseNewPM and invoke
>>> buildThinLTOPreLinkDefaultPipeline, but I'm not sure where.
>>>
>>
>> The LTOBackend is used at link time (the ThinLTO backends), and already
>> invokes buildThinLTODefaultPipeline. The passes in
>> buildThinLTOPreLinkDefaultPipeline are to be invoked during the compile
>> (pre-link) phase, and hence I would expect either clang side or via
>> something invoked to set up the compile passes from clang.
>>
>
> I'm betting we just didn't wire this up when Tim and I were going through.
>

It looks like clang invokes PassBuilder::buildPerModuleDefaultPipeline when
setting up the new pass manager, it should presumably be invoked from there
when we are doing a ThinLTO compile (or buildLTOPreLinkDefaultPipeline when
doing a regular LTO compile). Looks like these CodeGenOpts fields control
that (copying the code that detects whether we are compiling for ThinLTO or
regular LTO when setting up the legacy pipeline in clang):
   PMBuilder.PrepareForThinLTO = CodeGenOpts.EmitSummaryIndex;
   PMBuilder.PrepareForLTO = CodeGenOpts.PrepareForLTO;

Teresa

>
>
>>
>>
>>> I don't expect any more change on Clang side, though.
>>>
>>> On Tue, Jun 27, 2017 at 3:01 PM Chandler Carruth <chandlerc at google.com>
>>> wrote:
>>>
>>>> I thought Tim landed something that used this? But maybe
>>>> misremembering. I don't think I have a Clang patch outstanding... +Tim
>>>> Shen <timshen at google.com>
>>>>
>>>> On Tue, Jun 27, 2017 at 2:52 PM Teresa Johnson via llvm-commits <
>>>> llvm-commits at lists.llvm.org> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Thu, May 25, 2017, 2:43 PM Chandler Carruth via Phabricator <
>>>>> reviews at reviews.llvm.org> wrote:
>>>>>
>>>>>> chandlerc marked an inline comment as done.
>>>>>> chandlerc added a comment.
>>>>>>
>>>>>> In https://reviews.llvm.org/D33540#764460, @tejohnson wrote:
>>>>>>
>>>>>> > > I hoisted the ThinLTO stop point pre-link above the partial
>>>>>> inliner and the RPO function attr inference. Partial inlining isn't going
>>>>>> to make the code smaller and it seems better to just do that once post-link.
>>>>>> >
>>>>>> > Added davidxl to comment on this change. It seems like doing some
>>>>>> partial inlining during the pre-link for ThinLTO will have an effect on the
>>>>>> importing. Also, it is pre-link (as well as post-link) in the old pass
>>>>>> pipeline, so this would introduce an inconsistency between the pass
>>>>>> pipelines. May be better to keep it in the same places, and evaluate the
>>>>>> effect of removing it from the pre-link pass pipeline separately?
>>>>>>
>>>>>>
>>>>>> Well, partial inliner isn't run at all yet. So my hope was it can get
>>>>>> moved around later.
>>>>>>
>>>>>> I somewhat wanted to make the split line up with the simplification
>>>>>> split that is already there in the pipeline. But if you feel strongly we
>>>>>> need to keep these exactly the same, I can essentially copy the extra
>>>>>> passes into the tail of the thinlto prelink stuff.
>>>>>>
>>>>>>
>>>>>>
>>>>>> ================
>>>>>> Comment at: lib/Passes/PassBuilder.cpp:708
>>>>>> +ModulePassManager
>>>>>> +PassBuilder::buildThinLTOPreLinkDefaultPipeline(OptimizationLevel
>>>>>> Level,
>>>>>> +                                                bool DebugLogging) {
>>>>>> ----------------
>>>>>> tejohnson wrote:
>>>>>> > I don't see this getting invoked.
>>>>>> Yeah, the original patch had a change to Clang, but I wanted to get a
>>>>>> submittable LLVM-focused version.
>>>>>>
>>>>>
>>>>> Dehao just noticed that it still isn't be invoked, other than from the
>>>>> manual pipeline setup for testing. Do you still have your clang side
>>>>> changes around that you could submit? Or let me know if you prefer I add
>>>>> that.
>>>>> Teresa
>>>>>
>>>>>
>>>>>> I'll add support for parsing these pipelines much like we support
>>>>>> parsing 'default<O2>' and such so that we have some coverage of stuff.
>>>>>>
>>>>>>
>>>>>> https://reviews.llvm.org/D33540
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>
>>>>
>>
>>
>> --
>> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
>> 408-460-2413 <(408)%20460-2413>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>


-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170627/4ad326e5/attachment.html>