[llvm] r364422 - [InlineCost] cleanup calculations of Cost and Threshold
Fedor Sergeev via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 4 04:49:24 PDT 2019
Jordan,
if you could eventually share a reproducer that would be great!
thanks,
Fedor.
On 7/3/19 7:07 AM, Jordan Rupprecht wrote:
> Thanks, I've reverted in r365000 then.
>
> One of the crashes we noticed this in is actually open source
> (https://github.com/google/jsonnet/blob/master/core/parser.cpp),
> although I don't yet have a test driver for it to share.
>
> On Tue, Jul 2, 2019 at 8:12 PM Chandler Carruth <chandlerc at gmail.com
> <mailto:chandlerc at gmail.com>> wrote:
>
> I've spotted a serious bug in the patch, updated the review
> thread. Anyone should feel free to revert (I don't have a checkout
> handy or I would).
>
> On Tue, Jul 2, 2019 at 7:55 PM Chandler Carruth
> <chandlerc at gmail.com <mailto:chandlerc at gmail.com>> wrote:
>
> I think we should just revert this patch. The patch claims to
> not change behavior, but it clearly does. I'll follow up on
> the review thread with what I think is going wrong here.
>
> On Tue, Jul 2, 2019 at 5:05 PM Jordan Rupprecht via
> llvm-commits <llvm-commits at lists.llvm.org
> <mailto:llvm-commits at lists.llvm.org>> wrote:
>
> Hi Fedor,
>
> We're seeing some very unexpectedly large stack frame
> sizes as a result of this patch, e.g. from ~300 bytes to
> ~25kb. We're trying to reduce a test case to share. Is
> this kind of change expected with this patch?
>
> On Wed, Jun 26, 2019 at 6:24 AM Fedor Sergeev via
> llvm-commits <llvm-commits at lists.llvm.org
> <mailto:llvm-commits at lists.llvm.org>> wrote:
>
> Author: fedor.sergeev
> Date: Wed Jun 26 06:24:24 2019
> New Revision: 364422
>
> URL:
> http://llvm.org/viewvc/llvm-project?rev=364422&view=rev
> Log:
> [InlineCost] cleanup calculations of Cost and Threshold
>
> Summary:
> Doing better separation of Cost and Threshold.
> Cost counts the abstract complexity of live
> instructions, while Threshold is an upper bound of
> complexity that inlining is comfortable to pay.
> There are two parts:
> - huge 15K last-call-to-static bonus is no longer
> subtracted from Cost
> but rather is now added to Threshold.
>
> That makes much more sense, as the cost of
> inlining (Cost) is not changed by the fact
> that internal function is called once. It only
> changes the likelyhood of this inlining
> being profitable (Threshold).
>
> - bonus for calls proved-to-be-inlinable into
> callee is no longer subtracted from Cost
> but added to Threshold instead.
>
> While calculations are somewhat different, overall
> InlineResult should stay the same since Cost >=
> Threshold compares the same.
>
> Reviewers: eraman, greened, chandlerc, yrouban, apilipenko
> Reviewed By: apilipenko
> Tags: #llvm
> Differential Revision: https://reviews.llvm.org/D60740
>
> Modified:
> llvm/trunk/lib/Analysis/InlineCost.cpp
> llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks-with-hotness.ll
> llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks.ll
> llvm/trunk/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll
> llvm/trunk/test/LTO/X86/diagnostic-handler-remarks.ll
> llvm/trunk/test/Transforms/Inline/ARM/inline-fp.ll
>
> Modified: llvm/trunk/lib/Analysis/InlineCost.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InlineCost.cpp?rev=364422&r1=364421&r2=364422&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/InlineCost.cpp (original)
> +++ llvm/trunk/lib/Analysis/InlineCost.cpp Wed Jun 26
> 06:24:24 2019
> @@ -897,7 +897,15 @@ void
> CallAnalyzer::updateThreshold(CallB
> // and the callsite.
> int SingleBBBonusPercent = 50;
> int VectorBonusPercent = 150;
> - int LastCallToStaticBonus =
> InlineConstants::LastCallToStaticBonus;
> +
> + int LastCallToStaticBonus = 0;
> + bool OnlyOneCallAndLocalLinkage =
> + F.hasLocalLinkage() && F.hasOneUse() && &F ==
> Call.getCalledFunction();
> + // If there is only one call of the function, and
> it has internal linkage,
> + // we can allow to inline pretty anything as it
> will lead to size reduction
> + // anyway.
> + if (OnlyOneCallAndLocalLinkage)
> + LastCallToStaticBonus =
> InlineConstants::LastCallToStaticBonus;
>
> // Lambda to set all the above bonus and bonus
> percentages to 0.
> auto DisallowAllBonuses = [&]() {
> @@ -970,20 +978,13 @@ void
> CallAnalyzer::updateThreshold(CallB
> }
> }
>
> - // Finally, take the target-specific inlining
> threshold multiplier into
> - // account.
> + // Take the target-specific inlining threshold
> multiplier into account.
> Threshold *= TTI.getInliningThresholdMultiplier();
>
> SingleBBBonus = Threshold * SingleBBBonusPercent / 100;
> VectorBonus = Threshold * VectorBonusPercent / 100;
>
> - bool OnlyOneCallAndLocalLinkage =
> - F.hasLocalLinkage() && F.hasOneUse() && &F ==
> Call.getCalledFunction();
> - // If there is only one call of the function, and
> it has internal linkage,
> - // the cost of inlining it drops dramatically. It
> may seem odd to update
> - // Cost in updateThreshold, but the bonus depends
> on the logic in this method.
> - if (OnlyOneCallAndLocalLinkage)
> - Cost -= LastCallToStaticBonus;
> + Threshold += LastCallToStaticBonus;
> }
>
> bool CallAnalyzer::visitCmpInst(CmpInst &I) {
> @@ -1330,9 +1331,10 @@ bool
> CallAnalyzer::visitCallBase(CallBas
> CallAnalyzer CA(TTI, GetAssumptionCache, GetBFI,
> PSI, ORE, *F, Call,
> IndirectCallParams);
> if (CA.analyzeCall(Call)) {
> - // We were able to inline the indirect call!
> Subtract the cost from the
> - // threshold to get the bonus we want to apply,
> but don't go below zero.
> - Cost -= std::max(0, CA.getThreshold() -
> CA.getCost());
> + // We were able to inline the indirect call!
> Increase the threshold
> + // with the bonus we want to apply (less the cost
> of inlinee).
> + // Make sure the bonus doesn't go below zero.
> + Threshold += std::max(0, CA.getThreshold() -
> CA.getCost());
> }
>
> if (!F->onlyReadsMemory())
>
> Modified:
> llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks-with-hotness.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks-with-hotness.ll?rev=364422&r1=364421&r2=364422&view=diff
> ==============================================================================
> ---
> llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks-with-hotness.ll
> (original)
> +++
> llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks-with-hotness.ll
> Wed Jun 26 06:24:24 2019
> @@ -27,13 +27,13 @@
> ; YAML-NEXT: - Caller: main
> ; YAML-NEXT: - String: ' with '
> ; YAML-NEXT: - String: '(cost='
> -; YAML-NEXT: - Cost: '-15000'
> +; YAML-NEXT: - Cost: '0'
> ; YAML-NEXT: - String: ', threshold='
> -; YAML-NEXT: - Threshold: '337'
> +; YAML-NEXT: - Threshold: '15337'
> ; YAML-NEXT: - String: ')'
> ; YAML-NEXT: ...
>
> -; CHECK: tinkywinky inlined into main with
> (cost=-15000, threshold=337) (hotness: 300)
> +; CHECK: tinkywinky inlined into main with (cost=0,
> threshold=15337) (hotness: 300)
>
> target datalayout =
> "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> target triple = "x86_64-scei-ps4"
>
> Modified:
> llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks.ll?rev=364422&r1=364421&r2=364422&view=diff
> ==============================================================================
> ---
> llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks.ll
> (original)
> +++
> llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks.ll
> Wed Jun 26 06:24:24 2019
> @@ -30,9 +30,9 @@
> ; YAML-NEXT: - Caller: main
> ; YAML-NEXT: - String: ' with '
> ; YAML-NEXT: - String: '(cost='
> -; YAML-NEXT: - Cost: '-15000'
> +; YAML-NEXT: - Cost: '0'
> ; YAML-NEXT: - String: ', threshold='
> -; YAML-NEXT: - Threshold: '337'
> +; YAML-NEXT: - Threshold: '15337'
> ; YAML-NEXT: - String: ')'
> ; YAML-NEXT: ...
>
>
> Modified:
> llvm/trunk/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll?rev=364422&r1=364421&r2=364422&view=diff
> ==============================================================================
> ---
> llvm/trunk/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll
> (original)
> +++
> llvm/trunk/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll
> Wed Jun 26 06:24:24 2019
> @@ -19,9 +19,9 @@
> ; YAML-NEXT: - Caller: main
> ; YAML-NEXT: - String: ' with '
> ; YAML-NEXT: - String: '(cost='
> -; YAML-NEXT: - Cost: '-15000'
> +; YAML-NEXT: - Cost: '0'
> ; YAML-NEXT: - String: ', threshold='
> -; YAML-NEXT: - Threshold: '337'
> +; YAML-NEXT: - Threshold: '15337'
> ; YAML-NEXT: - String: ')'
> ; YAML-NEXT: ...
>
>
> Modified:
> llvm/trunk/test/LTO/X86/diagnostic-handler-remarks.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/LTO/X86/diagnostic-handler-remarks.ll?rev=364422&r1=364421&r2=364422&view=diff
> ==============================================================================
> ---
> llvm/trunk/test/LTO/X86/diagnostic-handler-remarks.ll
> (original)
> +++
> llvm/trunk/test/LTO/X86/diagnostic-handler-remarks.ll
> Wed Jun 26 06:24:24 2019
> @@ -55,9 +55,9 @@
> ; YAML-NEXT: - Caller: main
> ; YAML-NEXT: - String: ' with '
> ; YAML-NEXT: - String: '(cost='
> -; YAML-NEXT: - Cost: '-15000'
> +; YAML-NEXT: - Cost: '0'
> ; YAML-NEXT: - String: ', threshold='
> -; YAML-NEXT: - Threshold: '337'
> +; YAML-NEXT: - Threshold: '15337'
> ; YAML-NEXT: - String: ')'
> ; YAML-NEXT: ...
>
>
> Modified:
> llvm/trunk/test/Transforms/Inline/ARM/inline-fp.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/ARM/inline-fp.ll?rev=364422&r1=364421&r2=364422&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/Inline/ARM/inline-fp.ll
> (original)
> +++ llvm/trunk/test/Transforms/Inline/ARM/inline-fp.ll
> Wed Jun 26 06:24:24 2019
> @@ -7,7 +7,7 @@
> ; NOFP-DAG: single not inlined into test_single
> because too costly to inline (cost=125, threshold=75)
> ; NOFP-DAG: single not inlined into test_single
> because too costly to inline (cost=125, threshold=75)
> ; NOFP-DAG: single_cheap inlined into
> test_single_cheap with (cost=-15, threshold=75)
> -; NOFP-DAG: single_cheap inlined into
> test_single_cheap with (cost=-15015, threshold=75)
> +; NOFP-DAG: single_cheap inlined into
> test_single_cheap with (cost=-15, threshold=15075)
> ; NOFP-DAG: double not inlined into test_double
> because too costly to inline (cost=125, threshold=75)
> ; NOFP-DAG: double not inlined into test_double
> because too costly to inline (cost=125, threshold=75)
> ; NOFP-DAG: single_force_soft not inlined into
> test_single_force_soft because too costly to inline
> (cost=125, threshold=75)
> @@ -16,20 +16,20 @@
> ; NOFP-DAG: single_force_soft_fneg not inlined into
> test_single_force_soft_fneg because too costly to
> inline (cost=100, threshold=75)
>
> ; FULLFP-DAG: single inlined into test_single with
> (cost=0, threshold=75)
> -; FULLFP-DAG: single inlined into test_single with
> (cost=-15000, threshold=75)
> +; FULLFP-DAG: single inlined into test_single with
> (cost=0, threshold=15075)
> ; FULLFP-DAG: single_cheap inlined into
> test_single_cheap with (cost=-15, threshold=75)
> -; FULLFP-DAG: single_cheap inlined into
> test_single_cheap with (cost=-15015, threshold=75)
> +; FULLFP-DAG: single_cheap inlined into
> test_single_cheap with (cost=-15, threshold=15075)
> ; FULLFP-DAG: double inlined into test_double with
> (cost=0, threshold=75)
> -; FULLFP-DAG: double inlined into test_double with
> (cost=-15000, threshold=75)
> +; FULLFP-DAG: double inlined into test_double with
> (cost=0, threshold=15075)
> ; FULLFP-DAG: single_force_soft not inlined into
> test_single_force_soft because too costly to inline
> (cost=125, threshold=75)
> ; FULLFP-DAG: single_force_soft not inlined into
> test_single_force_soft because too costly to inline
> (cost=125, threshold=75)
> ; FULLFP-DAG: single_force_soft_fneg not inlined into
> test_single_force_soft_fneg because too costly to
> inline (cost=100, threshold=75)
> ; FULLFP-DAG: single_force_soft_fneg not inlined into
> test_single_force_soft_fneg because too costly to
> inline (cost=100, threshold=75)
>
> ; SINGLEFP-DAG: single inlined into test_single with
> (cost=0, threshold=75)
> -; SINGLEFP-DAG: single inlined into test_single with
> (cost=-15000, threshold=75)
> +; SINGLEFP-DAG: single inlined into test_single with
> (cost=0, threshold=15075)
> ; SINGLEFP-DAG: single_cheap inlined into
> test_single_cheap with (cost=-15, threshold=75)
> -; SINGLEFP-DAG: single_cheap inlined into
> test_single_cheap with (cost=-15015, threshold=75)
> +; SINGLEFP-DAG: single_cheap inlined into
> test_single_cheap with (cost=-15, threshold=15075)
> ; SINGLEFP-DAG: double not inlined into test_double
> because too costly to inline (cost=125, threshold=75)
> ; SINGLEFP-DAG: double not inlined into test_double
> because too costly to inline (cost=125, threshold=75)
> ; SINGLEFP-DAG: single_force_soft not inlined into
> test_single_force_soft because too costly to inline
> (cost=125, threshold=75)
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> <mailto:llvm-commits at lists.llvm.org>
> https://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>
> https://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/20190704/f07d7dbf/attachment-0001.html>
More information about the llvm-commits
mailing list