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

[Peter Collingbourne via llvm-commits]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180207/4d35f092/attachment.html>