<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<HTML><HEAD>
<META http-equiv=Content-Type content="text/html; charset=iso-8859-1">
<META content="MSHTML 6.00.6002.18591" name=GENERATOR></HEAD>
<BODY>
<DIV dir=ltr align=left><SPAN class=451223514-10052012><FONT face=Arial
color=#0000ff size=2>OK. I checked this a second time, and found that it wasn't
that hard to add support for opt -Os/-Oz, to mirror the clang
driver.</FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=451223514-10052012><FONT face=Arial
color=#0000ff size=2></FONT></SPAN> </DIV>
<DIV dir=ltr align=left><SPAN class=451223514-10052012><FONT face=Arial
color=#0000ff size=2>The attached patch contains</FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=451223514-10052012><FONT face=Arial
color=#0000ff size=2>* my previous patch for the
inliner,</FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=451223514-10052012><FONT face=Arial
color=#0000ff size=2>* the changes to the opt driver,
and</FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=451223514-10052012><FONT face=Arial
color=#0000ff size=2>* a testcase to test the inliner.</FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=451223514-10052012><FONT face=Arial
color=#0000ff size=2></FONT></SPAN> </DIV>
<DIV dir=ltr align=left><SPAN class=451223514-10052012><FONT face=Arial
color=#0000ff size=2>The opt -Os and -Oz changes has been tested to compile
and execute our own (quite small) collection of C
programs.</FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=451223514-10052012><FONT face=Arial
color=#0000ff size=2></FONT></SPAN> </DIV>
<DIV dir=ltr align=left><SPAN class=451223514-10052012><FONT face=Arial
color=#0000ff size=2>/Patrik Hägglund</FONT></SPAN></DIV><BR>
<DIV class=OutlookMessageHeader lang=en-us dir=ltr align=left>
<HR tabIndex=-1>
<FONT face=Tahoma size=2><B>From:</B> Chandler Carruth
[mailto:chandlerc@google.com] <BR><B>Sent:</B> den 8 maj 2012
09:20<BR><B>To:</B> Patrik Hägglund H<BR><B>Cc:</B> Bill Wendling;
<llvm-commits@cs.uiuc.edu><BR><B>Subject:</B> Re: [llvm-commits] [PATCH]
optsize attribute on top of -Oz gives bigger code<BR></FONT><BR></DIV>
<DIV></DIV>
<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="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid"><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><FONT face=Arial color=#0000ff size=2></FONT> </DIV>
<BLOCKQUOTE class=gmail_quote
style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid">
<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="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid">
<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></BODY></HTML>