Re: [PATCH] likely cleanup: remove unlikely for kfree(NULL)

[Bart Hartgers <bart@etpmod.phys.tue.nl>]

[-- Attachment #1: Type: text/plain, Size: 1936 bytes --]

Jörn Engel wrote:
> On Wed, 26 April 2006 13:03:34 +0200, Arjan van de Ven wrote:
>> On Wed, 2006-04-26 at 13:57 +0300, Pekka J Enberg wrote:
>>> On Wed, 26 April 2006 10:27:18 +0200, Arjan van de Ven wrote:
>>>>>> what I would like is kfree to become an inline wrapper that does the
>>>>>> null check inline, that way gcc can optimize it out (and it will in 4.1
>>>>>> with the VRP pass) if gcc can prove it's not NULL.
>>> On Wed, 2006-04-26 at 12:05 +0200, Jörn Engel wrote:
>>>>> How well can gcc optimize this case? 
>>> On Wed, 26 Apr 2006, Arjan van de Ven wrote:
>>>> if you deref'd the pointer it'll optimize it away (assuming a new enough
>>>> gcc, like 4.1)
>>> Here are the numbers for allyesconfig on my setup.
>>>
>>> 				Pekka
>>>
>>> gcc version 3.4.5 (Gentoo 3.4.5-r1, ssp-3.4.5-1.0, pie-8.7.9)
>> this is an ancient gcc without VRP so yeah the growth is expected ;)
> 
> In other words, we shouldn't do this as long as most users don't have
> gcc 4.1 or higher installed.  So this is somewhat pointless at the
> moment.
> 
> Still, if you could respin this with gcc 4.1 and post the numbers,
> Pekka, that would be quite interesting.
> 
> Jörn
> 

What about this:

static inline void my_kfree( void *ptr )
{
        if (__builtin_constant_p(ptr!=NULL)) {
                if (ptr!=NULL)
                        my_fast_free(ptr); /* skips NULL check */
        } else {
                my_checking_free(ptr); /* does a NULL check */
        }
}

That would skip the free when ptr is known to be NULL, and skip the
equal to NULL check if it is known to be not NULL, and do what happened
before otherwise. In other words, it is never worse than what we have now.

Attached is a small testcase in C and the resulting assembly. Note that
my compiler doesn't catch the "not equal to zero" case, but 4.1 is
supposed to do this.

Groeten,
Bart




-- 
Bart Hartgers - TUE Eindhoven - http://plasimo.phys.tue.nl/bart/contact/

[-- Attachment #2: testje.c --]
[-- Type: text/x-csrc, Size: 395 bytes --]

#define NULL ((void *)(0))

extern void my_fast_free(void *);
extern void my_checking_free(void *);

static inline void my_kfree( void *ptr )
{
	if (__builtin_constant_p(ptr!=NULL)) {
		if (ptr!=NULL)
			my_fast_free(ptr);
	} else {
		my_checking_free(ptr);
	}
}

void test( int *bla )
{
	char *hello = NULL;
	my_kfree(hello);
	my_kfree(bla);
	*bla = 1;	/* now it is !=NULL */
	my_kfree(bla);
}

[-- Attachment #3: testje.S --]
[-- Type: text/plain, Size: 418 bytes --]

	.file	"testje.c"
	.text
	.p2align 4,,15
.globl test
	.type	test, @function
test:
	pushl	%ebp
	movl	%esp, %ebp
	pushl	%ebx
	subl	$16, %esp
	movl	8(%ebp), %ebx
	pushl	%ebx
	call	my_checking_free
	movl	$1, (%ebx)
	movl	%ebx, 8(%ebp)
	addl	$16, %esp
	movl	-4(%ebp), %ebx
	leave
	jmp	my_checking_free
	.size	test, .-test
	.ident	"GCC: (GNU) 4.0.2 20050901 (prerelease) (SUSE Linux)"
	.section	.note.GNU-stack,"",@progbits