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

[Chad Rosier via llvm-commits]

Hi Peter,

On 4/6/2018 12:25 PM, Peter Collingbourne wrote:
> Unfortunately reverting would regress the user that requested this 
> patch. They use -plugin-opt=O0 and expect performance consistent with 
> setting cg opt level to default.

Using -O0 with LTO seems like a very odd/uncommon use case.  Can the 
user use '-mllvm -cg-opt-level=2' as an alternative solution to override 
the codegen optimization level used during LTO?

>
> Instead of reverting can we make a change that just sets the cg opt 
> level to aggressive if -plugin-opt=O3 is passed?

I'd prefer not.  IMO, this would just further muddy the water.  The 
'-mllvm -cg-opt-level=2' suggestion seems like a much less intrusive 
fix/workaround.

>
> Peter
>
>
> On Fri, Apr 6, 2018, 09:18 Chad Rosier <mcrosier at codeaurora.org 
> <mailto:mcrosier at codeaurora.org>> wrote:
>
>     Thanks, Rafael.  Sound reasonable to you, Peter?
>
>
>     On 4/6/2018 12:12 PM, Rafael Avila de Espindola wrote:
>     > I am OK with reverting it until we have a better framework in place.
>     >
>     > Cheers,
>     > Rafael
>     >
>     > Chad Rosier via llvm-commits <llvm-commits at lists.llvm.org
>     <mailto:llvm-commits at lists.llvm.org>> writes:
>     >
>     >> Peter/Rafael,
>     >>
>     >> Over the past few days I've been investigating several
>     regressions with
>     >> our weekly LTO configuration.  As a result, I identified that the
>     >> following regressions were due to this commit:
>     >>
>     >> SPEC2006/h264ref: -2.68%
>     >> SPEC2006/libquantum: -4.36%
>     >> SPEC2006/perlbench: -2.67%
>     >> SPEC2017/mcf: -4.47%
>     >> SPEC2017/perlbench: -3.82%
>     >>
>     >> I did not test SPEC2000, SPEC2006fp, or SPEC2017fp, but I'm seeing
>     >> similar small regressions in our weekly runs there as well
>     (i.e., the
>     >> above is just what I was able to test last night, but isn't
>     likely the
>     >> only regressions).  I'd like to kindly request that this change be
>     >> reverted until something is in place to properly propagate the
>     codegen
>     >> optimization level.
>     >>
>     >>    Chad
>     >>
>     >>
>     >> On 2/7/2018 9:41 PM, Peter Collingbourne via llvm-commits wrote:
>     >>> Author: pcc
>     >>> Date: Wed Feb  7 18:41:22 2018
>     >>> New Revision: 324557
>     >>>
>     >>> URL: http://llvm.org/viewvc/llvm-project?rev=324557&view=rev
>     >>> Log:
>     >>> gold-plugin: Do not set codegen opt level based on LTO opt level.
>     >>>
>     >>> 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.
>     >>>
>     >>> Differential Revision: https://reviews.llvm.org/D43040
>     >>>
>     >>> Modified:
>     >>>       llvm/trunk/tools/gold/gold-plugin.cpp
>     >>>
>     >>> Modified: llvm/trunk/tools/gold/gold-plugin.cpp
>     >>> URL:
>     http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/gold/gold-plugin.cpp?rev=324557&r1=324556&r2=324557&view=diff
>     >>>
>     ==============================================================================
>     >>> --- llvm/trunk/tools/gold/gold-plugin.cpp (original)
>     >>> +++ llvm/trunk/tools/gold/gold-plugin.cpp Wed Feb  7 18:41:22 2018
>     >>> @@ -727,20 +727,6 @@ static int getOutputFileName(StringRef I
>     >>>      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 @@ static std::unique_ptr<LTO> createLTO(In
>     >>>
>     >>>      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 <mailto:llvm-commits at lists.llvm.org>
>     >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>     >> _______________________________________________
>     >> llvm-commits mailing list
>     >> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>     >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180406/d0608834/attachment.html>