<div dir="ltr">(sorry for the dup, I forgot to update the list the first time....)<div><br></div><div>Wheeee commit necromancy!!!<br><br><div class="gmail_quote"><div dir="ltr">On Mon, May 19, 2014 at 11:36 AM Peter Collingbourne <<a href="mailto:peter@pcc.me.uk">peter@pcc.me.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: pcc<br>
Date: Mon May 19 13:25:54 2014<br>
New Revision: 209150<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=209150&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=209150&view=rev</a><br>
Log:<br>
Check the alwaysinline attribute on the call as well as on the caller.<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D3815" rel="noreferrer" target="_blank">http://reviews.llvm.org/D3815</a></blockquote><div><br></div><div><div>So, I reviewed this patch, and certainly the code is fine, but I wonder -- what was the motivation?</div><div><br></div><div>I was looking at this stuff today and I couldn't really understand why we would want any specific semantics for call-instruction "always inline" attributes.</div><div><br></div><div>I wonder if we could actually deprecate or in some way discourage frontends from emitting them.</div><div><br></div><div>In recent years, Richard Smith and Evgeniy have discussed moving always_inline toward a more rigorous model where it was invalid to place it on a function that we could not in fact inline and require that all such functions were in fact inlined. It turns out this in many ways better matches what at least the Clang frontend wants in many situations. It can emit a separate function definition to have its address taken and be used in other ways if strictly necessary.</div><div><br></div><div>But if we go that route, then always_inline really needs to be a property of the function and not the call instruction.</div><div><br></div><div>Thoughts? Thoughts from others?</div></div><div>-Chandler</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
Modified:<br>
    llvm/trunk/lib/Analysis/IPA/InlineCost.cpp<br>
    llvm/trunk/lib/Transforms/IPO/InlineAlways.cpp<br>
    llvm/trunk/test/Transforms/Inline/always-inline.ll<br>
<br>
Modified: llvm/trunk/lib/Analysis/IPA/InlineCost.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/IPA/InlineCost.cpp?rev=209150&r1=209149&r2=209150&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/IPA/InlineCost.cpp?rev=209150&r1=209149&r2=209150&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Analysis/IPA/InlineCost.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/IPA/InlineCost.cpp Mon May 19 13:25:54 2014<br>
@@ -1259,7 +1259,7 @@ InlineCost InlineCostAnalysis::getInline<br>
<br>
   // Calls to functions with always-inline attributes should be inlined<br>
   // whenever possible.<br>
-  if (Callee->hasFnAttribute(Attribute::AlwaysInline)) {<br>
+  if (CS.hasFnAttr(Attribute::AlwaysInline)) {<br>
     if (isInlineViable(*Callee))<br>
       return llvm::InlineCost::getAlways();<br>
     return llvm::InlineCost::getNever();<br>
<br>
Modified: llvm/trunk/lib/Transforms/IPO/InlineAlways.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/InlineAlways.cpp?rev=209150&r1=209149&r2=209150&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/InlineAlways.cpp?rev=209150&r1=209149&r2=209150&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/IPO/InlineAlways.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/IPO/InlineAlways.cpp Mon May 19 13:25:54 2014<br>
@@ -95,8 +95,7 @@ InlineCost AlwaysInliner::getInlineCost(<br>
   // that are viable for inlining. FIXME: We shouldn't even get here for<br>
   // declarations.<br>
   if (Callee && !Callee->isDeclaration() &&<br>
-      Callee->getAttributes().hasAttribute(AttributeSet::FunctionIndex,<br>
-                                           Attribute::AlwaysInline) &&<br>
+      CS.hasFnAttr(Attribute::AlwaysInline) &&<br>
       ICA->isInlineViable(*Callee))<br>
     return InlineCost::getAlways();<br>
<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=209150&r1=209149&r2=209150&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/always-inline.ll?rev=209150&r1=209149&r2=209150&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/Inline/always-inline.ll (original)<br>
+++ llvm/trunk/test/Transforms/Inline/always-inline.ll Mon May 19 13:25:54 2014<br>
@@ -122,3 +122,14 @@ entry:<br>
   ret void<br>
 }<br>
<br>
+define i32 @inner7() {<br>
+  ret i32 1<br>
+}<br>
+define i32 @outer7() {<br>
+; CHECK-LABEL: @outer7(<br>
+; CHECK-NOT: call<br>
+; CHECK: ret<br>
+<br>
+   %r = call i32 @inner7() alwaysinline<br>
+   ret i32 %r<br>
+}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div>