[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: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 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170627/1395c086/attachment.html>