<div dir="ltr"><div dir="ltr">r350751 adds an assert that the threshold cannot be negative. With this change, the commit description no longer applies.</div></div><br><div class="gmail_quote"><div dir="ltr">On Sun, Jan 6, 2019 at 9:56 PM David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@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">Could you add a test case for the negative cost situation you mentioned in the commit description?</div><br><div class="gmail_quote"><div dir="ltr">On Sat, Jan 5, 2019 at 1:30 PM Easwaran Raman via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">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: eraman<br>
Date: Fri Jan 4 18:26:29 2019<br>
New Revision: 350456<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=350456&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=350456&view=rev</a><br>
Log:<br>
[Inliner] Optimize shouldBeDeferred<br>
<br>
This has some minor optimizations to shouldBeDeferred. This is not<br>
strictly NFC because the early exit inside the loop assumes<br>
TotalSecondaryCost is monotonically non-decreasing, which is not true if<br>
the threshold used by CostAnalyzer is negative. AFAICT the thresholds do<br>
not go below 0 for the default values of the various options we use.<br>
<br>
Modified:<br>
llvm/trunk/lib/Transforms/IPO/Inliner.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/IPO/Inliner.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/Inliner.cpp?rev=350456&r1=350455&r2=350456&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/Inliner.cpp?rev=350456&r1=350455&r2=350456&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/IPO/Inliner.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/IPO/Inliner.cpp Fri Jan 4 18:26:29 2019<br>
@@ -311,6 +311,11 @@ shouldBeDeferred(Function *Caller, CallS<br>
// For now we only handle local or inline functions.<br>
if (!Caller->hasLocalLinkage() && !Caller->hasLinkOnceODRLinkage())<br>
return false;<br>
+ // If the cost of inlining CS is non-positive, it is not going to prevent the<br>
+ // caller from being inlined into its callers and hence we don't need to<br>
+ // defer.<br>
+ if (IC.getCost() <= 0)<br>
+ return false;<br>
// Try to detect the case where the current inlining candidate caller (call<br>
// it B) is a static or linkonce-ODR function and is an inlining candidate<br>
// elsewhere, and the current candidate callee (call it C) is large enough<br>
@@ -330,25 +335,31 @@ shouldBeDeferred(Function *Caller, CallS<br>
TotalSecondaryCost = 0;<br>
// The candidate cost to be imposed upon the current function.<br>
int CandidateCost = IC.getCost() - 1;<br>
- // This bool tracks what happens if we do NOT inline C into B.<br>
- bool callerWillBeRemoved = Caller->hasLocalLinkage();<br>
+ // If the caller has local linkage and can be inlined to all its callers, we<br>
+ // can apply a huge negative bonus to TotalSecondaryCost.<br>
+ bool ApplyLastCallBonus = Caller->hasLocalLinkage() && !Caller->hasOneUse();<br>
// This bool tracks what happens if we DO inline C into B.<br>
bool inliningPreventsSomeOuterInline = false;<br>
for (User *U : Caller->users()) {<br>
+ // If the caller will not be removed (either because it does not have a<br>
+ // local linkage or because the LastCallToStaticBonus has been already<br>
+ // applied), then we can exit the loop early.<br>
+ if (!ApplyLastCallBonus && TotalSecondaryCost >= IC.getCost())<br>
+ return false;<br>
CallSite CS2(U);<br>
<br>
// If this isn't a call to Caller (it could be some other sort<br>
// of reference) skip it. Such references will prevent the caller<br>
// from being removed.<br>
if (!CS2 || CS2.getCalledFunction() != Caller) {<br>
- callerWillBeRemoved = false;<br>
+ ApplyLastCallBonus = false;<br>
continue;<br>
}<br>
<br>
InlineCost IC2 = GetInlineCost(CS2);<br>
++NumCallerCallersAnalyzed;<br>
if (!IC2) {<br>
- callerWillBeRemoved = false;<br>
+ ApplyLastCallBonus = false;<br>
continue;<br>
}<br>
if (IC2.isAlways())<br>
@@ -366,7 +377,7 @@ shouldBeDeferred(Function *Caller, CallS<br>
// one is set very low by getInlineCost, in anticipation that Caller will<br>
// be removed entirely. We did not account for this above unless there<br>
// is only one caller of Caller.<br>
- if (callerWillBeRemoved && !Caller->hasOneUse())<br>
+ if (ApplyLastCallBonus)<br>
TotalSecondaryCost -= InlineConstants::LastCallToStaticBonus;<br>
<br>
if (inliningPreventsSomeOuterInline && TotalSecondaryCost < IC.getCost())<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>
</blockquote></div>