<div class="gmail_quote">On Mon, May 7, 2012 at 6:34 AM, Patrik Hägglund H <span dir="ltr"><<a href="mailto:patrik.h.hagglund@ericsson.com" target="_blank">patrik.h.hagglund@ericsson.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<u></u>



<div>
<div dir="ltr" align="left"><span><font face="Arial" color="#0000ff">Hi,</font></span></div>
<div dir="ltr" align="left"><span><font face="Arial" color="#0000ff"></font></span> </div>
<div dir="ltr" align="left"><span><font face="Arial" color="#0000ff">Any progress?</font></span></div></div></blockquote><div><br></div><div>Due to other pressing issues, no I've not had time to come up with anything. =/ Sorry. I don't know when I would have time as this isn't something likely to quickly return to my priority list. It will eventually, but you seem to have more immediate needs.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="im">
<div dir="ltr" align="left"><span><font face="Arial" color="#0000ff"></font></span> </div>
<div dir="ltr" align="left"><span><font face="Arial" color="#0000ff">I wrote:</font></span></div>> I can't see that any tool 
 in the llvm tree (specifically opt) is invoking that constructor.<span></span><br>
<div dir="ltr" align="left"><span><font face="Arial" color="#0000ff"></font></span> </div>
</div><div dir="ltr" align="left"><span><font face="Arial" color="#0000ff">Can you confirm or refute this?</font></span></div></div></blockquote><div><br></div><div>I've no idea, I'd have to go read a pile of code to figure it out. That said, if you've read the pile of code, I tend to believe you that we don't have such a tool today.</div>
<div><br></div><div>Maybe look at adding support for this to opt yourself if you've a need to enhance this aspect of LLVM? I think that's the fastest path toward progress.</div><div><br></div><div>-Chandler</div><div>
 </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>
<div dir="ltr" align="left"><span><font face="Arial" color="#0000ff"></font></span> </div>
<div dir="ltr" align="left"><span><font face="Arial" color="#0000ff">/Patrik Hägglund</font></span></div><br>
<div lang="en-us" dir="ltr" align="left">
<hr>
<font face="Tahoma"><div class="im"><b>From:</b> Chandler Carruth 
[mailto:<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>] <br></div><b>Sent:</b> den 30 april 2012 
12:21<div><div class="h5"><br><b>To:</b> Bill Wendling<br><b>Cc:</b> Patrik Hägglund H; 
<<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>><br><b>Subject:</b> Re: [llvm-commits] [PATCH] 
optsize attribute on top of -Oz gives bigger code<br></div></div></font><br></div><div><div class="h5">
<div></div>
<p>I agree, we need some testing of this logic. I can try to figure out how 
myself, but it will be a few days I suspect. </p>
<div class="gmail_quote">On Apr 30, 2012 3:19 AM, "Bill Wendling" <<a href="mailto:wendling@apple.com" target="_blank">wendling@apple.com</a>> wrote:<br type="attribution">
<blockquote class="gmail_quote" style="PADDING-LEFT:1ex;MARGIN:0px 0px 0px 0.8ex;BORDER-LEFT:#ccc 1px solid">Sorry. 
  I think if you can cobble together some test cases, then it would be 
  good.<br><br>-bw<br><br>On Apr 30, 2012, at 12:52 AM, Patrik Hägglund H 
  wrote:<br><br>> Is there anything more I can do to make this patch 
  accepted?<br>><br>> /Patrik Hägglund<br>><br>> From: Patrik 
  Hägglund H<br>> Sent: den 26 april 2012 15:33<br>> To: 'Chandler 
  Carruth'; Bill Wendling<br>> Cc: <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>> 
  Subject: RE: [llvm-commits] [PATCH] optsize attribute on top of -Oz gives 
  bigger code<br>><br>> Sorry for the late reply.<br>><br>> > I 
  like the patch in general. I have only one high-level comment: it needs test 
  cases. ;] They should be pretty  easy to rig up with custom 
  thresholds.<br>><br>> My patch is for the case when the second 
  constructor for the Inliner class, Inliner(char &ID, int Threshold, bool 
  InsertLifetime), is invoked (with a low value for Threshold). Clang invokes 
  that constructor, but I can't see that any tool  in the llvm tree 
  (specifically opt) is invoking that constructor. Therefore, I can't see how to 
  write a test case (inside llvm/test).<br>> /Patrik Hägglund<br>><br>> 
  From: Chandler Carruth [mailto:<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>]<br>> Sent: den 
  5 april 2012 00:45<br>> To: Bill Wendling<br>> Cc: Patrik Hägglund H; <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>> 
  Subject: Re: [llvm-commits] [PATCH] optsize attribute on top of -Oz gives 
  bigger code<br>><br>> On Wed, Apr 4, 2012 at 11:17 PM, Bill Wendling 
  <<a href="mailto:wendling@apple.com" target="_blank">wendling@apple.com</a>> 
  wrote:<br>> Hi Patrik,<br>><br>> Thank you for running the tests. The 
  patch looks fine to me. Chandler may comment on the patch once it's in. If so, 
  you can now argue that it's beneficial. :-)<br>><br>> I heard that. 
  ;]<br>><br>> I like the patch in general. I have only one high-level 
  comment: it needs test cases. ;] They should be pretty easy to rig up with 
  custom thresholds.<br>><br>> I think there is more work to be done here, 
  but this is a strict improvement and gives a nice lift, so LGTM! 
  =D<br>><br>><br>> -bw<br>><br>> On Apr 4, 2012, at 6:14 AM, 
  Patrik Hägglund H wrote:<br>><br>> > Hi Bill,<br>> ><br>> 
  > I have now taken the time to run the test suite on r153939 from 
  yesterday. The total object size went from 9779419 down to 9634074, when my 
  patch was applied.<br>> ><br>> > In both builds, I replaced 
  'OPTFLAGS := -O3' with 'OPTFLAGS := -Oz' in Makefile.rules in the test-suite 
  directory, and executed 'make TEST=simple'. I then ran 'size -t `find -name 
  \*.simple`'. The diff is attached.<br>> ><br>> > BTW, is there any 
  build bot logs that can be used to see the current status for test suite 
  executions? For example, I didn't got the sphereflake test to pass (and one 
  other test, which I have forgotten).<br>> ><br>> > Here is the 
  updated patch:<br>> ><br>> > diff --git 
  a/lib/Transforms/IPO/Inliner.cpp b/lib/Transforms/IPO/Inliner.cpp<br>> > 
  index 8a9d149..4e13596 100644<br>> > --- 
  a/lib/Transforms/IPO/Inliner.cpp<br>> > +++ 
  b/lib/Transforms/IPO/Inliner.cpp<br>> > @@ -196,19 +196,23 @@ static 
  bool InlineCallIfPossible(CallSite CS, InlineFunctionInfo &IFI,<br>> 
  > }<br>> ><br>> > unsigned Inliner::getInlineThreshold(CallSite 
  CS) const {<br>> > -  int thres = InlineThreshold;<br>> > + 
   int thres = InlineThreshold; // -inline-threshold or else selected 
  by<br>> > +                 
                // overall opt level<br>> 
  ><br>> > -  // Listen to optsize when -inline-limit is not 
  given.<br>> > +  // If -inline-threshold is not given, listen to 
  the optsize attribute when it<br>> > +  // would decrease the 
  threshold.<br>> >   Function *Caller = CS.getCaller();<br>> > 
  -  if (Caller && !Caller->isDeclaration() &&<br>> 
  > -      Caller->hasFnAttr(Attribute::OptimizeForSize) 
  &&<br>> > +  bool OptSize = Caller && 
  !Caller->isDeclaration() &&<br>> > +   
   Caller->hasFnAttr(Attribute::OptimizeForSize);<br>> > + 
   if (OptSize && OptSizeThreshold < thres &&<br>> 
  >       InlineLimit.getNumOccurrences() == 0)<br>> > 
      thres = OptSizeThreshold;<br>> ><br>> > -  // 
  Listen to inlinehint when it would increase the threshold.<br>> > + 
   // Listen to the inlinehint attribute when it would increase the 
  threshold.<br>> >   Function *Callee = 
  CS.getCalledFunction();<br>> > -  if (HintThreshold > thres 
  && Callee && !Callee->isDeclaration() &&<br>> 
  > -     
   Callee->hasFnAttr(Attribute::InlineHint))<br>> > +  bool 
  InlineHint = Callee && !Callee->isDeclaration() &&<br>> 
  > +    Callee->hasFnAttr(Attribute::InlineHint);<br>> > 
  +  if (InlineHint && HintThreshold > thres)<br>> > 
      thres = HintThreshold;<br>> ><br>> >   return 
  thres;<br>> ><br>> > /Patrik Hägglund<br>> ><br>> > 
  -----Original Message-----<br>> > From: Patrik Hägglund H<br>> > 
  Sent: den 22 mars 2012 08:57<br>> > To: 'Bill Wendling'<br>> > Cc: 
  <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>> 
  > Subject: RE: [llvm-commits] [PATCH] optsize attribute on top of -Oz gives 
  bigger code<br>> ><br>> >> Could you give some metrics to go 
  along with the patch? You should be able to measure the impact on the LLVM 
  test suite.<br>> ><br>> > Well, the few times I have tried to 
  execute the test suite, it have failed in one way or another. (On SLED 11 and 
  Debian unstable). But maybe its time to give it another try.<br>> 
  ><br>> > BTW, what's the purpose of the metrics? My thinking was that 
  this is more an issue about how you define the optsize attribute. In my 
  opinion adding optisize should not make any impact  on the code size when 
  using clang -Os or -Oz. At least the code should not be larger, but not 
  smaller either. That is, the code should be identical. That is what this patch 
  tries to fix (with the regard to the inliner). If the code shouldn't be 
  identical, exactly what is the definiton of the optsize attribute, relative to 
  clang -Os and -Oz (or other potential users of the LLVM libraries passing 
  similar information)?<br>> ><br>> >> A comment on the patch 
  itself:<br>> ><br>> > I like consistency. The logic mirrors the 
  order in the comment, and InlineLimit.getNumOccurrences() > 0 is used 
  because it is also used in the constructor. But I can change that part if you 
  like.<br>> ><br>> > /Patrik Hägglund<br>> ><br>> > 
  -----Original Message-----<br>> > From: Bill Wendling [mailto:<a href="mailto:wendling@apple.com" target="_blank">wendling@apple.com</a>]<br>> > Sent: 
  den 22 mars 2012 08:10<br>> > To: Patrik Hägglund H<br>> > Cc: <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>> 
  > Subject: Re: [llvm-commits] [PATCH] optsize attribute on top of -Oz gives 
  bigger code<br>> ><br>> > Hi Patrik,<br>> ><br>> > The 
  patch looks okay overall. Could you give some metrics to go along with the 
  patch? You should be able to measure the impact on the LLVM test 
  suite.<br>> ><br>> > A comment on the patch itself:<br>> 
  ><br>> > +  if (!(InlineLimit.getNumOccurrences() > 0) 
  && OptSize && OptSizeThreshold < thres)<br>> 
  ><br>> > The logic could be made better (and more readable). Please 
  use this instead:<br>> ><br>> > +  if (OptSize && 
  OptSizeThreshold < thres && InlineLimit.getNumOccurrences() == 
  0)<br>> ><br>> > -bw<br>> ><br>> > On Mar 20, 2012, at 
  6:30 AM, Patrik Hägglund H wrote:<br>> ><br>> >> Hi,<br>> 
  >><br>> >> I found that using clang -Oz, used as a backend for 
  .ll files, combined with using the optsize attribute, then optsize _increased_ 
  the code size.<br>> >><br>> >> The base inline threshold was 
  25 with clang -Oz, but 75 when adding the optsize attribute.<br>> 
  >><br>> >> This patch fix this to only change the inline 
  threshold for a function with optsize if it lowers the base threshold.<br>> 
  >><br>> >> /Patrik Hägglund<br>> >><br>> >> 
  <optsize_inliner.diff>_______________________________________________<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" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>> 
  ><br>> > <size.diff><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" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br><br></blockquote></div></div></div></div>
</blockquote></div><br>