[llvm] r364422 - [InlineCost] cleanup calculations of Cost and Threshold
Jordan Rupprecht via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 2 21:07:47 PDT 2019
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> 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>
> 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/42b427ba/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4849 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190702/42b427ba/attachment.bin>
More information about the llvm-commits
mailing list