<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 15, 2013 at 9:31 AM, Diego Novillo <span dir="ltr"><<a href="mailto:dnovillo@google.com" target="_blank" class="cremed">dnovillo@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im">On 2013-04-10 17:33 , Diego Novillo wrote:<br>

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
This patch adds a new function attribute 'cold' to functions.<br>
</blockquote>
<br></div>
Thanks for all the feedback.  In fixing up the patch I noticed that there is a 'bindings/' directory where attributes seem to play a role.  I'm not sure what this is.  Not all the existing attributes seem to exist in these bindings.<br>
</blockquote><div><br></div><div style>Yea, our bindings are spottily maintained.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

<br>
Should I add the following changes as well?  There don't seem to be tests for these files (nothing I could find in a quick search, anyway).<br></blockquote><div><br></div><div style>I would add them, but maybe in a separate commit.</div>
<div style> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Main patch is attached.  OK to commit?</blockquote></div><div class="gmail_extra"><br></div>LGTM. Check w/ Duncan though since he looked at one version of it.</div><div class="gmail_extra"><br></div><div class="gmail_extra" style>
Some comments about the documentation, which don't need to be for this patch necessarily....</div><div class="gmail_extra" style><br></div><div class="gmail_extra" style><pre style="color:rgb(0,0,0);word-wrap:break-word;white-space:pre-wrap">
diff --git a/docs/LangRef.rst b/docs/LangRef.rst
index 4d006f1..1c0864d 100644
--- a/docs/LangRef.rst
+++ b/docs/LangRef.rst
@@ -807,6 +807,11 @@ example:
     This attribute indicates that the inliner should attempt to inline
     this function into callers whenever possible, ignoring any active
     inlining size threshold for this caller.
+``cold``
+    This attribute indicates that this function is not called
+    frequently.  When computing edge weights, basic blocks
+    post-dominated by a cold function are also considered to be cold;
+    and, thus, given low weight.</pre>I feel like we should give users a bit more guidance about this attribute, what semantics we ascribe to it, and under what circumstance it would be appropriate to use. Specifically, I worry about users thinking they should use the attribute on a call which happens "only" 30% of the time.</div>
<div class="gmail_extra" style><br></div><div class="gmail_extra" style>The way I would like to clarify this is to say that the attribute indicates that the performance of code in that path should be prioritized below any other performance concerns in the program. Thus, programs should only put this attribute on code paths which they are fine running with an arbitrarily large (while bounded) overhead.</div>
<div class="gmail_extra" style><br></div><div class="gmail_extra" style>I'm not sure what the best way to phrase all of this is though...</div></div>