[PATCH] D43040: gold-plugin: Do not set codegen opt level based on LTO opt level.

[Rafael Avila de Espindola via llvm-commits]

LGTM

Peter Collingbourne <peter at pcc.me.uk> writes:

> In general we should try to allow users to control compilation settings at
> compile time, however I don't think that settings that affect the linked
> program as a whole should fall into this category. Some examples of this
> are linker flags for performing whole-program size optimizations (e.g.
> --icf, --gc-sections, -O). Another example is the LTO opt level, which
> essentially controls the amount of cross-module optimization done by LTO.
> The backend operates on a per-function basis, so the work that it does is
> unrelated to cross-module optimization and therefore should not be
> controlled by the LTO opt level but rather (ideally) function-level
> attributes. You can imagine future extensions to the backend that could
> cause it to operate on a cross-module basis, such as interprocedural
> register allocation, but I think those should again be controlled by the
> LTO opt level.
>
> Peter
>
> On Wed, Feb 7, 2018 at 4:27 PM, Rafael Avila de Espindola <
> rafael.espindola at gmail.com> wrote:
>
>> It sounds reasonable. It is also nice to have lld and gold agree on
>> this, but could expand a bit on why it is a bad idea to set it on the
>> linker?
>>
>> Cheers,
>> Rafael
>>
>> Peter Collingbourne <peter at pcc.me.uk> writes:
>>
>> > We use CodeGenOpt::Default (i.e. 2).
>> >
>> > Peter
>> >
>> > On Wed, Feb 7, 2018 at 3:44 PM, Rafael Avila de Espindola <
>> > rafael.espindola at gmail.com> wrote:
>> >
>> >> What level is used without a IR attribute or Conf.CGOptLevel being set?
>> >>
>> >> Cheers,
>> >> Rafael
>> >>
>> >> Peter Collingbourne via Phabricator via llvm-commits
>> >> <llvm-commits at lists.llvm.org> writes:
>> >>
>> >> > pcc created this revision.
>> >> > pcc added reviewers: tejohnson, eugenis.
>> >> > Herald added subscribers: inglorion, mehdi_amini.
>> >> >
>> >> > The LTO opt level should not affect the codegen opt level, and indeed
>> >> > it does not affect it in lld. Ideally the codegen opt level should
>> >> > be controlled by an IR-level attribute based on the compile-time opt
>> >> > level, but that hasn't been implemented yet.
>> >> >
>> >> >
>> >> > https://reviews.llvm.org/D43040
>> >> >
>> >> > Files:
>> >> >   llvm/tools/gold/gold-plugin.cpp
>> >> >
>> >> >
>> >> > Index: llvm/tools/gold/gold-plugin.cpp
>> >> > ===================================================================
>> >> > --- llvm/tools/gold/gold-plugin.cpp
>> >> > +++ llvm/tools/gold/gold-plugin.cpp
>> >> > @@ -727,20 +727,6 @@
>> >> >    return FD;
>> >> >  }
>> >> >
>> >> > -static CodeGenOpt::Level getCGOptLevel() {
>> >> > -  switch (options::OptLevel) {
>> >> > -  case 0:
>> >> > -    return CodeGenOpt::None;
>> >> > -  case 1:
>> >> > -    return CodeGenOpt::Less;
>> >> > -  case 2:
>> >> > -    return CodeGenOpt::Default;
>> >> > -  case 3:
>> >> > -    return CodeGenOpt::Aggressive;
>> >> > -  }
>> >> > -  llvm_unreachable("Invalid optimization level");
>> >> > -}
>> >> > -
>> >> >  /// Parse the thinlto_prefix_replace option into the \p OldPrefix and
>> >> >  /// \p NewPrefix strings, if it was specified.
>> >> >  static void getThinLTOOldAndNewPrefix(std::string &OldPrefix,
>> >> > @@ -767,7 +753,6 @@
>> >> >
>> >> >    Conf.MAttrs = MAttrs;
>> >> >    Conf.RelocModel = RelocationModel;
>> >> > -  Conf.CGOptLevel = getCGOptLevel();
>> >> >    Conf.DisableVerify = options::DisableVerify;
>> >> >    Conf.OptLevel = options::OptLevel;
>> >> >    if (options::Parallelism)
>> >> >
>> >> >
>> >> > Index: llvm/tools/gold/gold-plugin.cpp
>> >> > ===================================================================
>> >> > --- llvm/tools/gold/gold-plugin.cpp
>> >> > +++ llvm/tools/gold/gold-plugin.cpp
>> >> > @@ -727,20 +727,6 @@
>> >> >    return FD;
>> >> >  }
>> >> >
>> >> > -static CodeGenOpt::Level getCGOptLevel() {
>> >> > -  switch (options::OptLevel) {
>> >> > -  case 0:
>> >> > -    return CodeGenOpt::None;
>> >> > -  case 1:
>> >> > -    return CodeGenOpt::Less;
>> >> > -  case 2:
>> >> > -    return CodeGenOpt::Default;
>> >> > -  case 3:
>> >> > -    return CodeGenOpt::Aggressive;
>> >> > -  }
>> >> > -  llvm_unreachable("Invalid optimization level");
>> >> > -}
>> >> > -
>> >> >  /// Parse the thinlto_prefix_replace option into the \p OldPrefix and
>> >> >  /// \p NewPrefix strings, if it was specified.
>> >> >  static void getThinLTOOldAndNewPrefix(std::string &OldPrefix,
>> >> > @@ -767,7 +753,6 @@
>> >> >
>> >> >    Conf.MAttrs = MAttrs;
>> >> >    Conf.RelocModel = RelocationModel;
>> >> > -  Conf.CGOptLevel = getCGOptLevel();
>> >> >    Conf.DisableVerify = options::DisableVerify;
>> >> >    Conf.OptLevel = options::OptLevel;
>> >> >    if (options::Parallelism)
>> >> > _______________________________________________
>> >> > llvm-commits mailing list
>> >> > llvm-commits at lists.llvm.org
>> >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> >>
>> >
>> >
>> >
>> > --
>> > --
>> > Peter
>>
>
>
>
> -- 
> -- 
> Peter