<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Jordan,<br>
<br>
if you could eventually share a reproducer that would be great!<br>
<br>
thanks,<br>
Fedor.<br>
<br>
<div class="moz-cite-prefix">On 7/3/19 7:07 AM, Jordan Rupprecht
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CABC7LbR7p4D409JNURCb=hCrDjBRuJfK6qhHd0uaFej1btEv=A@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div dir="ltr">Thanks, I've reverted in r365000 then.
<div><br>
</div>
<div>One of the crashes we noticed this in is actually open
source (<a
href="https://github.com/google/jsonnet/blob/master/core/parser.cpp"
class="cremed" moz-do-not-send="true">https://github.com/google/jsonnet/blob/master/core/parser.cpp</a>),
although I don't yet have a test driver for it to share.</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue, Jul 2, 2019 at 8:12 PM
Chandler Carruth <<a href="mailto:chandlerc@gmail.com"
moz-do-not-send="true">chandlerc@gmail.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr">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).</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue, Jul 2, 2019 at
7:55 PM Chandler Carruth <<a
href="mailto:chandlerc@gmail.com" target="_blank"
moz-do-not-send="true">chandlerc@gmail.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div dir="ltr">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.</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue, Jul 2, 2019 at
5:05 PM Jordan Rupprecht via llvm-commits <<a
href="mailto:llvm-commits@lists.llvm.org"
target="_blank" moz-do-not-send="true">llvm-commits@lists.llvm.org</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div dir="ltr">Hi Fedor,
<div><br>
<div>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?</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Wed, Jun 26,
2019 at 6:24 AM Fedor Sergeev via llvm-commits
<<a href="mailto:llvm-commits@lists.llvm.org"
target="_blank" moz-do-not-send="true">llvm-commits@lists.llvm.org</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">Author:
fedor.sergeev<br>
Date: Wed Jun 26 06:24:24 2019<br>
New Revision: 364422<br>
<br>
URL: <a
href="http://llvm.org/viewvc/llvm-project?rev=364422&view=rev"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://llvm.org/viewvc/llvm-project?rev=364422&view=rev</a><br>
Log:<br>
[InlineCost] cleanup calculations of Cost and
Threshold<br>
<br>
Summary:<br>
Doing better separation of Cost and Threshold.<br>
Cost counts the abstract complexity of live
instructions, while Threshold is an upper bound of
complexity that inlining is comfortable to pay.<br>
There are two parts:<br>
- huge 15K last-call-to-static bonus is no
longer subtracted from Cost<br>
but rather is now added to Threshold.<br>
<br>
That makes much more sense, as the cost of
inlining (Cost) is not changed by the fact<br>
that internal function is called once. It
only changes the likelyhood of this inlining<br>
being profitable (Threshold).<br>
<br>
- bonus for calls proved-to-be-inlinable into
callee is no longer subtracted from Cost<br>
but added to Threshold instead.<br>
<br>
While calculations are somewhat different,
overall InlineResult should stay the same since
Cost >= Threshold compares the same.<br>
<br>
Reviewers: eraman, greened, chandlerc, yrouban,
apilipenko<br>
Reviewed By: apilipenko<br>
Tags: #llvm<br>
Differential Revision: <a
href="https://reviews.llvm.org/D60740"
rel="noreferrer" target="_blank"
moz-do-not-send="true">https://reviews.llvm.org/D60740</a><br>
<br>
Modified:<br>
llvm/trunk/lib/Analysis/InlineCost.cpp<br>
llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks-with-hotness.ll<br>
llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks.ll<br>
llvm/trunk/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll<br>
llvm/trunk/test/LTO/X86/diagnostic-handler-remarks.ll<br>
llvm/trunk/test/Transforms/Inline/ARM/inline-fp.ll<br>
<br>
Modified: llvm/trunk/lib/Analysis/InlineCost.cpp<br>
URL: <a
href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InlineCost.cpp?rev=364422&r1=364421&r2=364422&view=diff"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InlineCost.cpp?rev=364422&r1=364421&r2=364422&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Analysis/InlineCost.cpp
(original)<br>
+++ llvm/trunk/lib/Analysis/InlineCost.cpp Wed Jun
26 06:24:24 2019<br>
@@ -897,7 +897,15 @@ void
CallAnalyzer::updateThreshold(CallB<br>
// and the callsite.<br>
int SingleBBBonusPercent = 50;<br>
int VectorBonusPercent = 150;<br>
- int LastCallToStaticBonus =
InlineConstants::LastCallToStaticBonus;<br>
+<br>
+ int LastCallToStaticBonus = 0;<br>
+ bool OnlyOneCallAndLocalLinkage =<br>
+ F.hasLocalLinkage() &&
F.hasOneUse() && &F ==
Call.getCalledFunction();<br>
+ // If there is only one call of the function,
and it has internal linkage,<br>
+ // we can allow to inline pretty anything as it
will lead to size reduction<br>
+ // anyway.<br>
+ if (OnlyOneCallAndLocalLinkage)<br>
+ LastCallToStaticBonus =
InlineConstants::LastCallToStaticBonus;<br>
<br>
// Lambda to set all the above bonus and bonus
percentages to 0.<br>
auto DisallowAllBonuses = [&]() {<br>
@@ -970,20 +978,13 @@ void
CallAnalyzer::updateThreshold(CallB<br>
}<br>
}<br>
<br>
- // Finally, take the target-specific inlining
threshold multiplier into<br>
- // account.<br>
+ // Take the target-specific inlining threshold
multiplier into account.<br>
Threshold *=
TTI.getInliningThresholdMultiplier();<br>
<br>
SingleBBBonus = Threshold *
SingleBBBonusPercent / 100;<br>
VectorBonus = Threshold * VectorBonusPercent /
100;<br>
<br>
- bool OnlyOneCallAndLocalLinkage =<br>
- F.hasLocalLinkage() &&
F.hasOneUse() && &F ==
Call.getCalledFunction();<br>
- // If there is only one call of the function,
and it has internal linkage,<br>
- // the cost of inlining it drops dramatically.
It may seem odd to update<br>
- // Cost in updateThreshold, but the bonus
depends on the logic in this method.<br>
- if (OnlyOneCallAndLocalLinkage)<br>
- Cost -= LastCallToStaticBonus;<br>
+ Threshold += LastCallToStaticBonus;<br>
}<br>
<br>
bool CallAnalyzer::visitCmpInst(CmpInst &I) {<br>
@@ -1330,9 +1331,10 @@ bool
CallAnalyzer::visitCallBase(CallBas<br>
CallAnalyzer CA(TTI, GetAssumptionCache,
GetBFI, PSI, ORE, *F, Call,<br>
IndirectCallParams);<br>
if (CA.analyzeCall(Call)) {<br>
- // We were able to inline the indirect call!
Subtract the cost from the<br>
- // threshold to get the bonus we want to
apply, but don't go below zero.<br>
- Cost -= std::max(0, CA.getThreshold() -
CA.getCost());<br>
+ // We were able to inline the indirect call!
Increase the threshold<br>
+ // with the bonus we want to apply (less the
cost of inlinee).<br>
+ // Make sure the bonus doesn't go below zero.<br>
+ Threshold += std::max(0, CA.getThreshold() -
CA.getCost());<br>
}<br>
<br>
if (!F->onlyReadsMemory())<br>
<br>
Modified:
llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks-with-hotness.ll<br>
URL: <a
href="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"
rel="noreferrer" target="_blank"
moz-do-not-send="true">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</a><br>
==============================================================================<br>
---
llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks-with-hotness.ll
(original)<br>
+++
llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks-with-hotness.ll
Wed Jun 26 06:24:24 2019<br>
@@ -27,13 +27,13 @@<br>
; YAML-NEXT: - Caller: main<br>
; YAML-NEXT: - String: ' with '<br>
; YAML-NEXT: - String: '(cost='<br>
-; YAML-NEXT: - Cost: '-15000'<br>
+; YAML-NEXT: - Cost: '0'<br>
; YAML-NEXT: - String: ', threshold='<br>
-; YAML-NEXT: - Threshold: '337'<br>
+; YAML-NEXT: - Threshold: '15337'<br>
; YAML-NEXT: - String: ')'<br>
; YAML-NEXT: ...<br>
<br>
-; CHECK: tinkywinky inlined into main with
(cost=-15000, threshold=337) (hotness: 300)<br>
+; CHECK: tinkywinky inlined into main with
(cost=0, threshold=15337) (hotness: 300)<br>
<br>
target datalayout =
"e-m:e-i64:64-f80:128-n8:16:32:64-S128"<br>
target triple = "x86_64-scei-ps4"<br>
<br>
Modified:
llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks.ll<br>
URL: <a
href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks.ll?rev=364422&r1=364421&r2=364422&view=diff"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks.ll?rev=364422&r1=364421&r2=364422&view=diff</a><br>
==============================================================================<br>
---
llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks.ll
(original)<br>
+++
llvm/trunk/test/LTO/Resolution/X86/diagnostic-handler-remarks.ll
Wed Jun 26 06:24:24 2019<br>
@@ -30,9 +30,9 @@<br>
; YAML-NEXT: - Caller: main<br>
; YAML-NEXT: - String: ' with '<br>
; YAML-NEXT: - String: '(cost='<br>
-; YAML-NEXT: - Cost: '-15000'<br>
+; YAML-NEXT: - Cost: '0'<br>
; YAML-NEXT: - String: ', threshold='<br>
-; YAML-NEXT: - Threshold: '337'<br>
+; YAML-NEXT: - Threshold: '15337'<br>
; YAML-NEXT: - String: ')'<br>
; YAML-NEXT: ...<br>
<br>
<br>
Modified:
llvm/trunk/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll<br>
URL: <a
href="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"
rel="noreferrer" target="_blank"
moz-do-not-send="true">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</a><br>
==============================================================================<br>
---
llvm/trunk/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll
(original)<br>
+++
llvm/trunk/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll
Wed Jun 26 06:24:24 2019<br>
@@ -19,9 +19,9 @@<br>
; YAML-NEXT: - Caller: main<br>
; YAML-NEXT: - String: ' with '<br>
; YAML-NEXT: - String: '(cost='<br>
-; YAML-NEXT: - Cost: '-15000'<br>
+; YAML-NEXT: - Cost: '0'<br>
; YAML-NEXT: - String: ', threshold='<br>
-; YAML-NEXT: - Threshold: '337'<br>
+; YAML-NEXT: - Threshold: '15337'<br>
; YAML-NEXT: - String: ')'<br>
; YAML-NEXT: ...<br>
<br>
<br>
Modified:
llvm/trunk/test/LTO/X86/diagnostic-handler-remarks.ll<br>
URL: <a
href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/LTO/X86/diagnostic-handler-remarks.ll?rev=364422&r1=364421&r2=364422&view=diff"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/LTO/X86/diagnostic-handler-remarks.ll?rev=364422&r1=364421&r2=364422&view=diff</a><br>
==============================================================================<br>
---
llvm/trunk/test/LTO/X86/diagnostic-handler-remarks.ll
(original)<br>
+++
llvm/trunk/test/LTO/X86/diagnostic-handler-remarks.ll
Wed Jun 26 06:24:24 2019<br>
@@ -55,9 +55,9 @@<br>
; YAML-NEXT: - Caller: main<br>
; YAML-NEXT: - String: ' with '<br>
; YAML-NEXT: - String: '(cost='<br>
-; YAML-NEXT: - Cost: '-15000'<br>
+; YAML-NEXT: - Cost: '0'<br>
; YAML-NEXT: - String: ', threshold='<br>
-; YAML-NEXT: - Threshold: '337'<br>
+; YAML-NEXT: - Threshold: '15337'<br>
; YAML-NEXT: - String: ')'<br>
; YAML-NEXT: ...<br>
<br>
<br>
Modified:
llvm/trunk/test/Transforms/Inline/ARM/inline-fp.ll<br>
URL: <a
href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/ARM/inline-fp.ll?rev=364422&r1=364421&r2=364422&view=diff"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/ARM/inline-fp.ll?rev=364422&r1=364421&r2=364422&view=diff</a><br>
==============================================================================<br>
---
llvm/trunk/test/Transforms/Inline/ARM/inline-fp.ll
(original)<br>
+++
llvm/trunk/test/Transforms/Inline/ARM/inline-fp.ll
Wed Jun 26 06:24:24 2019<br>
@@ -7,7 +7,7 @@<br>
; NOFP-DAG: single not inlined into test_single
because too costly to inline (cost=125,
threshold=75)<br>
; NOFP-DAG: single not inlined into test_single
because too costly to inline (cost=125,
threshold=75)<br>
; NOFP-DAG: single_cheap inlined into
test_single_cheap with (cost=-15, threshold=75)<br>
-; NOFP-DAG: single_cheap inlined into
test_single_cheap with (cost=-15015, threshold=75)<br>
+; NOFP-DAG: single_cheap inlined into
test_single_cheap with (cost=-15, threshold=15075)<br>
; NOFP-DAG: double not inlined into test_double
because too costly to inline (cost=125,
threshold=75)<br>
; NOFP-DAG: double not inlined into test_double
because too costly to inline (cost=125,
threshold=75)<br>
; NOFP-DAG: single_force_soft not inlined into
test_single_force_soft because too costly to
inline (cost=125, threshold=75)<br>
@@ -16,20 +16,20 @@<br>
; NOFP-DAG: single_force_soft_fneg not inlined
into test_single_force_soft_fneg because too
costly to inline (cost=100, threshold=75)<br>
<br>
; FULLFP-DAG: single inlined into test_single
with (cost=0, threshold=75)<br>
-; FULLFP-DAG: single inlined into test_single
with (cost=-15000, threshold=75)<br>
+; FULLFP-DAG: single inlined into test_single
with (cost=0, threshold=15075)<br>
; FULLFP-DAG: single_cheap inlined into
test_single_cheap with (cost=-15, threshold=75)<br>
-; FULLFP-DAG: single_cheap inlined into
test_single_cheap with (cost=-15015, threshold=75)<br>
+; FULLFP-DAG: single_cheap inlined into
test_single_cheap with (cost=-15, threshold=15075)<br>
; FULLFP-DAG: double inlined into test_double
with (cost=0, threshold=75)<br>
-; FULLFP-DAG: double inlined into test_double
with (cost=-15000, threshold=75)<br>
+; FULLFP-DAG: double inlined into test_double
with (cost=0, threshold=15075)<br>
; FULLFP-DAG: single_force_soft not inlined into
test_single_force_soft because too costly to
inline (cost=125, threshold=75)<br>
; FULLFP-DAG: single_force_soft not inlined into
test_single_force_soft because too costly to
inline (cost=125, threshold=75)<br>
; FULLFP-DAG: single_force_soft_fneg not inlined
into test_single_force_soft_fneg because too
costly to inline (cost=100, threshold=75)<br>
; FULLFP-DAG: single_force_soft_fneg not inlined
into test_single_force_soft_fneg because too
costly to inline (cost=100, threshold=75)<br>
<br>
; SINGLEFP-DAG: single inlined into test_single
with (cost=0, threshold=75)<br>
-; SINGLEFP-DAG: single inlined into test_single
with (cost=-15000, threshold=75)<br>
+; SINGLEFP-DAG: single inlined into test_single
with (cost=0, threshold=15075)<br>
; SINGLEFP-DAG: single_cheap inlined into
test_single_cheap with (cost=-15, threshold=75)<br>
-; SINGLEFP-DAG: single_cheap inlined into
test_single_cheap with (cost=-15015, threshold=75)<br>
+; SINGLEFP-DAG: single_cheap inlined into
test_single_cheap with (cost=-15, threshold=15075)<br>
; SINGLEFP-DAG: double not inlined into
test_double because too costly to inline
(cost=125, threshold=75)<br>
; SINGLEFP-DAG: double not inlined into
test_double because too costly to inline
(cost=125, threshold=75)<br>
; SINGLEFP-DAG: single_force_soft not inlined
into test_single_force_soft because too costly to
inline (cost=125, threshold=75)<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org"
target="_blank" moz-do-not-send="true">llvm-commits@lists.llvm.org</a><br>
<a
href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits"
rel="noreferrer" target="_blank"
moz-do-not-send="true">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote>
</div>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org"
target="_blank" moz-do-not-send="true">llvm-commits@lists.llvm.org</a><br>
<a
href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits"
rel="noreferrer" target="_blank"
moz-do-not-send="true">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote>
</div>
</blockquote>
</div>
</blockquote>
</div>
</blockquote>
<br>
</body>
</html>