[llvm] r364422 - [InlineCost] cleanup calculations of Cost and Threshold
Chandler Carruth via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 2 20:12:36 PDT 2019
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> 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> 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> 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
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>> _______________________________________________
>> llvm-commits mailing list
>> 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/20190702/2260c828/attachment.html>
More information about the llvm-commits
mailing list