[llvm] r324557 - gold-plugin: Do not set codegen opt level based on LTO opt level.
Chad Rosier via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 6 12:03:59 PDT 2018
Hi Peter,
Unfortunately, it's difficult for me to chime in on what is or is not
the right approach to solving this problem; I'm not a domain expert for
LTO. What you're suggesting does make sense to me (i.e., adding the LTO
APIs as a temporary stop gap) and I assure you I very much want to
accommodate you and your customer's request.
However, after this commit if a users requests -O3 for both the
compilation and link steps with LTO that's not what they get. While I'm
not a domain expert, that does seem very non-intuitive to me. Moreover,
this commit resulted in a large number of performance regressions across
a wide suite of benchmarks. In such scenarios, the general practice in
the community has been to revert such changes immediately and then have
the necessary discussion to move forward, not the other way around.
This is my request and I would still like to see this commit reverted
per community standard practices.
Unfortunately, I don't have the time or resources to address this issue
myself (as I initially began investigating in D45225/D45226). My only
request is that whatever solution is decided upon that it doesn't cause
avoidable regressions.
Chad
On 4/6/2018 1:37 PM, Peter Collingbourne wrote:
> On Fri, Apr 6, 2018 at 10:15 AM, Chad Rosier <mcrosier at codeaurora.org
> <mailto:mcrosier at codeaurora.org>> wrote:
>
> 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.
>
>
> It can be used to get program transformations that rely on
> whole-program information (such as control flow integrity
> <https://clang.llvm.org/docs/ControlFlowIntegrity.html>) without
> requesting cross-module optimizations. This is important for users who
> want CFI but do not want to pay the link time or code size cost of
> cross-module optimizations.
>
> Can the user use '-mllvm -cg-opt-level=2' as an alternative
> solution to override the codegen optimization level used during LTO?
>
>
> This flag is not present in the gold plugin, only in llvm-lto2.
>
>
>
>>
>> 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.
>
>
> Adding the flag would be going in the wrong direction IMHO. What we
> should be working towards is removing the LTO configuration option
> entirely. We can do that by moving the default/aggressive selection
> into the LTO API implementation so that the behaviour is consistent
> between linkers. Then once the attribute is ready we can remove that code.
>
> Peter
>
>
>
>>
>> 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
>> <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
>> <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
>> <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
>> <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
>> <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>>
>
>
>
>
> --
> --
> Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180406/7575c0f5/attachment.html>
More information about the llvm-commits
mailing list