[llvm-commits] [PATCH] Skip NULL pointer check for free

Quentin Colombet qcolombet at apple.com
Wed Jan 2 11:42:20 PST 2013


Hi Chandler,

Thanks for the comments.

Like you pointed out, the patch can really hurt the performance in some cases.

Hence, I've changed it so that the transformation is done only when Oz (i.e., MinSize function attribute) is set on the caller function.

New patch attached.

-Quentin


On Jan 2, 2013, at 11:15 AM, Chandler Carruth <chandlerc at google.com> wrote:

> I really do share Nick's concern.
> 
> I would be fine doing this in -Oz mode, where hurting performance for code size is the goal, but outside of that I'm deeply skeptical that we have enough information to make the right decision here.
> 
> There are code patterns where the pointer is almost-always null. Imagine a sparsely populated vector of pointers. In this case, there will be a hot loop where you are turning a highly predictable branch into a function call.
> 
> We could use various "hotness" heuristics from the BlockFrequencyInfo to control this optimization, but honestly I think the only one that we won't find to pessimize real world code is "very cold" relative to the function entry block.
> 
> On Wed, Jan 2, 2013 at 10:18 AM, Quentin Colombet <qcolombet at apple.com> wrote:
> Ping!
> -Quentin
> 
> 
> 
> On Dec 21, 2012, at 4:04 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> 
>> Hi Nick,
>> 
>> On Dec 21, 2012, at 3:59 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
>> 
>>> On Sat, Dec 22, 2012 at 1:53 AM, Nick Lewycky <nicholas at mxc.ca> wrote:
>>>> On 12/21/2012 03:43 PM, Quentin Colombet wrote:
>>>>> 
>>>>> Hi,
>>>>> 
>>>>> Here is a patch to help llvm turning a code like this:
>>>>> if (foo)
>>>>> free(foo)
>>>>> 
>>>>> into that:
>>>>> free(foo)
>>>>> 
>>>>> It is legal and safe, since free is already checking its argument
>>>>> against NULL internally (I may find the reference in the Standard if
>>>>> needed :)).
>>>> 
>>>> It's legal and safe, but is it desirable? Calling a function is expensive
>>>> even if the function doesn't do anything.
>> 
>> Yes, you're right, it may not be desirable but I'm tempt to think like Dmitri.
>> 
>> Do you think that this optimization should be controlled by a flag?
>> At least, for code size (Oz and maybe Os), it may always be desirable.
>> 
>> I did not consider changing free(foo) into if(foo) free(foo)
>> 
>>> 
>>> That's true, but there's another viewpoint: programmers usually write
>>> 'if(!foo) free(foo);' not because they are manually inlining the fast
>>> path of free(), but because they think that free(NULL) is UB.
>>> 
>>> Dmitri
>>> 
>>> -- 
>>> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
>>> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130102/884e3dca/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: skipNullPtrCheckForFreeFct.patch
Type: application/octet-stream
Size: 5976 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130102/884e3dca/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130102/884e3dca/attachment-0001.html>


More information about the llvm-commits mailing list