<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Jun 12, 2017 at 6:34 PM Chandler Carruth <<a href="mailto:chandlerc@gmail.com">chandlerc@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Jun 12, 2017 at 4:04 PM David Blaikie 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hey Chandler - wouldn't mind a touch of post-commit review here (just in case this wouldn't've otherwise crossed your dashboard).<br><br>Specifically this moves the shouldInline call in the non-trivial case to before the InlineHistory lookup. I'm wondering if that could have a significant performance impact such that it'd be worth consulting the InlineHistory before the shouldInline test.<br><br>(similarly, I don't have a great sense for whether shouldInline might be substantially more expensive than 'isInstructionTriviallyDead' in case it'd be better for those to be in the opposite order)<br></div></blockquote><div><br></div></div></div><div dir="ltr"><div class="gmail_quote"><div>shouldInline is very, very expensive. Please check the inline history first, and check for anything like trivially dead instructions first.</div></div></div></blockquote><div><br>Ah, thanks!<br><br>r305267<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br>This is currently the tidiest way to write this - but if those orderings are likely to be important I'm happy to refactor it to address them.<br><br>- Dave</div><div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Jun 12, 2017 at 4:01 PM David Blaikie 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: dblaikie<br>
Date: Mon Jun 12 18:01:17 2017<br>
New Revision: 305245<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=305245&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=305245&view=rev</a><br>
Log:<br>
Inliner: Don't remove calls to readnone+nounwind (but not always_inline) functions in the AlwaysInliner<br>
<br>
Modified:<br>
    llvm/trunk/lib/Transforms/IPO/Inliner.cpp<br>
    llvm/trunk/test/Transforms/Inline/always-inline.ll<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=305245&r1=305244&r2=305245&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/Inliner.cpp?rev=305245&r1=305244&r2=305245&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/IPO/Inliner.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/IPO/Inliner.cpp Mon Jun 12 18:01:17 2017<br>
@@ -523,6 +523,16 @@ inlineCallsImpl(CallGraphSCC &SCC, CallG<br>
       if (!Callee || Callee->isDeclaration())<br>
         continue;<br>
<br>
+      // FIXME for new PM: because of the old PM we currently generate ORE and<br>
+      // in turn BFI on demand.  With the new PM, the ORE dependency should<br>
+      // just become a regular analysis dependency.<br>
+      OptimizationRemarkEmitter ORE(Caller);<br>
+<br>
+      // If the policy determines that we should inline this function,<br>
+      // delete the call instead.<br>
+      if (!shouldInline(CS, GetInlineCost, ORE))<br>
+        continue;<br>
+<br>
       // If this call site is dead and it is to a readonly function, we should<br>
       // just delete the call instead of trying to inline it, regardless of<br>
       // size.  This happens because IPSCCP propagates the result out of the<br>
@@ -548,15 +558,6 @@ inlineCallsImpl(CallGraphSCC &SCC, CallG<br>
         // Get DebugLoc to report. CS will be invalid after Inliner.<br>
         DebugLoc DLoc = CS.getInstruction()->getDebugLoc();<br>
         BasicBlock *Block = CS.getParent();<br>
-        // FIXME for new PM: because of the old PM we currently generate ORE and<br>
-        // in turn BFI on demand.  With the new PM, the ORE dependency should<br>
-        // just become a regular analysis dependency.<br>
-        OptimizationRemarkEmitter ORE(Caller);<br>
-<br>
-        // If the policy determines that we should inline this function,<br>
-        // try to do so.<br>
-        if (!shouldInline(CS, GetInlineCost, ORE))<br>
-          continue;<br>
<br>
         // Attempt to inline the function.<br>
         using namespace ore;<br>
<br>
Modified: llvm/trunk/test/Transforms/Inline/always-inline.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/always-inline.ll?rev=305245&r1=305244&r2=305245&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/always-inline.ll?rev=305245&r1=305244&r2=305245&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/Inline/always-inline.ll (original)<br>
+++ llvm/trunk/test/Transforms/Inline/always-inline.ll Mon Jun 12 18:01:17 2017<br>
@@ -305,3 +305,14 @@ entry:<br>
   ret void<br>
 ; CHECK: ret void<br>
 }<br>
+<br>
+define void @inner14() readnone nounwind {<br>
+; CHECK: define void @inner14<br>
+  ret void<br>
+}<br>
+<br>
+define void @outer14() {<br>
+; CHECK: call void @inner14<br>
+  call void @inner14()<br>
+  ret void<br>
+}<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></div>
_______________________________________________<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></div></blockquote></div></div>