<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">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;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>