[llvm-commits] [PATCH] optsize attribute on top of -Oz gives bigger code

[Bill Wendling]

On Mar 22, 2012, at 12:56 AM, Patrik Hägglund H wrote:

>> Could you give some metrics to go along with the patch? You should be able to measure the impact on the LLVM test suite.
> 
> 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.
> 
> 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)?
> 
You said in your message "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". I would like to see metrics that shows that your patch doesn't increase the code size in these cases. The point being that there aren't any tests that go along with your patch (nor do I think there could be). So we need to make sure your patch does what you say it does before we declare the problem solved.

>> A comment on the patch itself:
> 
> 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.
> 
It's nicer to do the cheap tests first. Doing something like "!(x > 0)" is using negative logic which is hard for humans to follow.

-bw

> -----Original Message-----
> From: Bill Wendling [mailto:wendling at apple.com] 
> Sent: den 22 mars 2012 08:10
> To: Patrik Hägglund H
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [PATCH] optsize attribute on top of -Oz gives bigger code
> 
> Hi Patrik,
> 
> 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.
> 
> A comment on the patch itself:
> 
> +  if (!(InlineLimit.getNumOccurrences() > 0) && OptSize && OptSizeThreshold < thres)
> 
> The logic could be made better (and more readable). Please use this instead:
> 
> +  if (OptSize && OptSizeThreshold < thres && InlineLimit.getNumOccurrences() == 0)
> 
> -bw
> 
> On Mar 20, 2012, at 6:30 AM, Patrik Hägglund H wrote:
> 
>> Hi,
>> 
>> 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.
>> 
>> The base inline threshold was 25 with clang -Oz, but 75 when adding the optsize attribute.
>> 
>> This patch fix this to only change the inline threshold for a function with optsize if it lowers the base threshold.
>> 
>> /Patrik Hägglund
>> 
>> <optsize_inliner.diff>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>