[PATCH] D11908: Clang support for -fthinlto.

[Duncan P. N. Exon Smith via cfe-commits]

+echristo, +espindola, +pcc

> On 2015-Sep-30, at 12:39, Teresa Johnson <tejohnson at google.com> wrote:
> 
> On Wed, Sep 30, 2015 at 11:11 AM, Duncan P. N. Exon Smith
> <dexonsmith at apple.com> wrote:
>> 
>>> On 2015-Sep-30, at 10:40, Teresa Johnson <tejohnson at google.com> wrote:
>>> 
>>> On Wed, Sep 30, 2015 at 10:19 AM, Duncan P. N. Exon Smith
>>> <dexonsmith at apple.com> wrote:
>>>> 
>>>>> On 2015-Sep-23, at 10:28, Teresa Johnson <tejohnson at google.com> wrote:
>>>>> 
>>>>> Index: include/clang/Driver/Options.td
>>>>> ===================================================================
>>>>> --- include/clang/Driver/Options.td
>>>>> +++ include/clang/Driver/Options.td
>>>>> @@ -686,6 +686,9 @@
>>>>> def flto_EQ : Joined<["-"], "flto=">, Group<clang_ignored_gcc_optimization_f_Group>;
>>>>> def flto : Flag<["-"], "flto">, Flags<[CC1Option]>, Group<f_Group>;
>>>>> def fno_lto : Flag<["-"], "fno-lto">, Group<f_Group>;
>>>>> +def fthinlto : Flag<["-"], "fthinlto">, Flags<[CC1Option]>, Group<f_Group>;
>>>> 
>>>> I wonder whether this is the right way to add new variants of LTO.
>>>> I.e., maybe this should be "-flto=thin"?  Do you happen to know how this
>>>> would conflict with GCC options?  (I see we ignore "-flto="...)
>>> 
>>> I looked at GCC docs and sources. It does have a -flto= variant. From
>>> https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html:
>>> -----------------
>>> If you specify the optional n, the optimization and code generation
>>> done at link time is executed in parallel using n parallel jobs by
>>> utilizing an installed make program. The environment variable MAKE may
>>> be used to override the program used. The default value for n is 1.
>>> 
>>> You can also specify -flto=jobserver to use GNU make's job server mode
>>> to determine the number of parallel jobs. This is useful when the
>>> Makefile calling GCC is already executing in parallel. You must
>>> prepend a ‘+’ to the command recipe in the parent Makefile for this to
>>> work. This option likely only works if MAKE is GNU make.
>>> ----------------
>>> 
>>> I would anticipate that we may want to support something like this for
>>> ThinLTO eventually to specify the number of parallel jobs for the
>>> ThinLTO backend processes. So it might be confusing to overload
>>> -flto=.
>> 
>> Hmm.  You're right that the GCC usage seems pretty different.
>> 
>> I guess you're envisioning -fthinlto=jobserver?
> 
> Or -fthinlto=n.
> 
>> I wonder if we could
>> do this as -flto=thin,jobserver or something similar?
> 
> I am ok with -flto=thin and then later extending to -flto=thin,n etc.
> 
> This simplifies the interaction with -fno-lto.
> 
> I think a -flto=thin followed by -flto should probably revert to
> normal LTO, WDYT?
> 
> Another thing to consider is to add -flto=full or something like that
> which is currently an alias for -flto. So we would have:
> 
> -flto=<type> where type could be "full" or "thin"
> -flto means -flto=full
> -fno-lto disables all types of flto
> and last option of the above 3 wins.

This SGTM.  "monolithic" might be more descriptive than "full", or
maybe someone has a better name.

Eric/Rafael/Peter: any thoughts about driver options?

>> 
>> It's pretty hard to remove driver options, so I think it's important to
>> get the interface right.  I anticipate that we'll add more LTO variants
>> in the future, so we should take care that this scales reasonably.
>> 
>> (This may be a good discussion for cfe-dev, not sure.)
>> 
>>>> E.g., as a user I'd expect -fno-lto to turn off whatever LTO was turned
>>>> on.  And I might expect -flto to override -fthinlto, or vice versa.
>>> 
>>> Good point, I think the last one should win. Looks like right now
>>> -fthinlto is effectively always winning. I will fix that.
>>> 
>>> Not sure about -fno-lto behavior if we leave -fthinlto as a separate
>>> option. Let me know what you think based on the GCC usage of -flto=.
>> 
>> Right, if they're totally separate, then either behaviour for -fno-lto
>> is confusing.