Messages in this thread Patch in this message | | | Date | Wed, 26 Apr 2006 15:04:29 +0200 | From | Bart Hartgers <> | Subject | Re: [PATCH] likely cleanup: remove unlikely for kfree(NULL) |
| |
Bart Hartgers wrote: > 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 >
Sorry about replying to my own mail, but I discovered that at least "gcc (GCC) 4.1.0 (SUSE Linux)" does not seem to combine the delete-null-pointer optimisation with the builtin_constant test. The compiler is happy to eliminate ptr==NULL tests, but does not consider the expression (ptr==NULL) constant! I managed to hack around this.
See the attached code, and:
bart@gum15:~> gcc -DCASE_A -m32 -O3 -S -o testje-a.S testje.c bart@gum15:~> gcc -DCASE_B -m32 -O3 -S -o testje-b.S testje.c bart@gum15:~> diff -u testje-a.S testje-b.S --- testje-a.S 2006-04-26 14:57:50.000000000 +0200 +++ testje-b.S 2006-04-26 14:57:53.000000000 +0200 @@ -16,7 +16,7 @@ addl $4, %esp popl %ebx popl %ebp - jmp my_fast_free + jmp my_slow_free .size test, .-test .ident "GCC: (GNU) 4.1.0 (SUSE Linux)" .section .note.GNU-stack,"",@progbits Anyway, CASE_A produces optimal code for gcc 4.1, and gcc 4.0 produces identical code in both cases.
Groeten, Bart -- Bart Hartgers - TUE Eindhoven - http://plasimo.phys.tue.nl/bart/contact/ #include <stddef.h>
extern void my_fast_free(void *); extern void my_checking_free(void *);
static inline void my_kfree( void *ptr ) { #ifdef CASE_A register int is_null = 0; if (ptr == NULL) is_null = 1; if (__builtin_constant_p(is_null)) { #else /* CASE_B */ if (__builtin_constant_p(ptr==NULL)) { #endif if (ptr != NULL) my_fast_free(ptr); } else { my_slow_free(ptr); } }
void test( int *bla ) { char *hello = NULL; my_kfree(hello); my_kfree(bla); *bla = 1; my_kfree(bla); } .file "testje.c" .text .p2align 4,,15 .globl test .type test, @function test: pushl %ebp movl %esp, %ebp pushl %ebx subl $4, %esp movl 8(%ebp), %ebx movl %ebx, (%esp) call my_slow_free movl $1, (%ebx) movl %ebx, 8(%ebp) addl $4, %esp popl %ebx popl %ebp jmp my_fast_free .size test, .-test .ident "GCC: (GNU) 4.1.0 (SUSE Linux)" .section .note.GNU-stack,"",@progbits
| |