[llvm] r364422 - [InlineCost] cleanup calculations of Cost and Threshold
Chandler Carruth via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 2 19:55:25 PDT 2019
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/ac8fe86e/attachment.html>
More information about the llvm-commits
mailing list