Re: [PATCH] 2.4.22pre10: {,un}likely_p() macros for pointers

Re: [PATCH] 2.4.22pre10: {,un}likely_p() macros for pointers

[Albert Cahalan]

Chip Salzenberg writes:

> GCC is warning about a pointer-to-int conversion when
> the likely() and unlikely() macros are used with pointer
> values.  So, for architectures where pointers are larger
> than 'int', I suggest this patch.
...
> -#define likely(x) __builtin_expect((x),1)
> -#define unlikely(x) __builtin_expect((x),0)
> +#define likely(x) __builtin_expect((x),      1)
> +#define likely_p(x) __builtin_expect((x) != 0, 1)
> +#define unlikely(x) __builtin_expect((x)      ,0)
> +#define unlikely_p(x) __builtin_expect((x) != 0 ,0)

That's ugly, plus the "_p" suffix is kind of a
standard for "predicate". (__builtin_constant_p, etc.)

I'm using these in the procps project:

// tell gcc what to expect:   if(unlikely(err)) die(err);
#define likely(x)       __builtin_expect(!!(x),1)
#define unlikely(x)     __builtin_expect(!!(x),0)
#define expected(x,y)   __builtin_expect((x),(y))

That makes a slight change to the meaning, since the
original value is no longer available. I've not
found that to be any trouble at all; if it is then
you could work around it using a statement-expression
with a variable, cast, and/or __typeof__.

Something like this:

#define likely(x) ({   \
__typeof__ (x) _tmp;    \
__builtin_expect(!!_tmp,1); \
_tmp; \
})

Re: [PATCH] 2.4.22pre10: {,un}likely_p() macros for pointers

[Jamie Lokier]

Albert Cahalan wrote:
> // tell gcc what to expect:   if(unlikely(err)) die(err);
> #define likely(x)       __builtin_expect(!!(x),1)
> #define unlikely(x)     __builtin_expect(!!(x),0)
> #define expected(x,y)   __builtin_expect((x),(y))

You may want to check that GCC generates the same code as for

	#define likely(x)	__builtin_expect((x),1)
	#define unlikely(x)	__builtin_expect((x),0)

I tried this once, and I don't recall the precise result but I do
recall it generating different code (i.e. not optimal for one of the
definitions).

-- Jamie

Re: [PATCH] 2.4.22pre10: {,un}likely_p() macros for pointers

[Willy Tarreau]

On Sat, Aug 09, 2003 at 01:21:17AM +0100, Jamie Lokier wrote:
> Albert Cahalan wrote:
> > // tell gcc what to expect:   if(unlikely(err)) die(err);
> > #define likely(x)       __builtin_expect(!!(x),1)
> > #define unlikely(x)     __builtin_expect(!!(x),0)
> > #define expected(x,y)   __builtin_expect((x),(y))
> 
> You may want to check that GCC generates the same code as for
> 
> 	#define likely(x)	__builtin_expect((x),1)
> 	#define unlikely(x)	__builtin_expect((x),0)
> 
> I tried this once, and I don't recall the precise result but I do
> recall it generating different code (i.e. not optimal for one of the
> definitions).

anyway, in __builtin_expect(!!(x),0) there is a useless conversion, because
it's totally equivalent to __builtin_expect((x),0) (how could !!x be 0 if x
isn't ?), but I'm pretty sure that the compiler will add the test.

Willy

Re: [PATCH] 2.4.22pre10: {,un}likely_p() macros for pointers

[David S. Miller]

On Sat, 9 Aug 2003 10:13:46 +0200
Willy Tarreau <willy@w.ods.org> wrote:

> (how could !!x be 0 if x isn't ?)

I believe the C language allows for systems where the NULL pointer is
not zero.

I can't think of any reason why the NULL macro exists otherwise.

However, even if I'm right, I dread the guy who has to make other
people's code work on such a platform.  Using normal boolean tests for
NULL pointer checks is just too common.

Re: [PATCH] 2.4.22pre10: {,un}likely_p() macros for pointers

[Jamie Lokier]

David S. Miller wrote:
> On Sat, 9 Aug 2003 10:13:46 +0200 Willy Tarreau <willy@w.ods.org> wrote:
> 
> > (how could !!x be 0 if x isn't ?)
> 
> I believe the C language allows for systems where the NULL pointer is
> not zero.

That is irrelevant.  The GCC manual says you can't use a pointer as
the argument to __builtin_expect anyway:

     Since you are limited to integral expressions for EXP, you should
     use constructions such as

          if (__builtin_expect (ptr != NULL, 1))
            error ();

     when testing pointer or floating-point values

-- Jamie

Re: [PATCH] 2.4.22pre10: {,un}likely_p() macros for pointers

[Herbert Xu]

David S. Miller <davem@redhat.com> wrote:
> 
> I believe the C language allows for systems where the NULL pointer is
> not zero.

It does.  However it also says that whenever a pointer is compared
with the integer constant 0 it must be equal if and only if it is
a NULL pointer.

    An integer constant expression with the value 0, or such an expression
    cast to type void *, is called a null pointer constant.
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] 2.4.22pre10: {,un}likely_p() macros for pointers

[Alan Cox]

On Sad, 2003-08-09 at 09:51, David S. Miller wrote:
> I believe the C language allows for systems where the NULL pointer is
> not zero.

Its a fudge for some systems. However NULL == 0 is always true.

> I can't think of any reason why the NULL macro exists otherwise.

<OldFart>
NULL is really important in K&R C because you don't have prototypes and
sizeof(foo *) may not be the same as sizeof(int). This leads to very
nasty problems that people nowdays forget about.
</OldFart>

Re: [PATCH] 2.4.22pre10: {,un}likely_p() macros for pointers

[Jamie Lokier]

Alan Cox wrote:
> > I believe the C language allows for systems where the NULL pointer is
> > not zero.

The representation is not zero, however in the C language (since ANSI,
not sure about K&R), 0 is a valid name for the NULL pointer whatever
its representation.

> > I can't think of any reason why the NULL macro exists otherwise.
> 
> <OldFart>
> NULL is really important in K&R C because you don't have prototypes and
> sizeof(foo *) may not be the same as sizeof(int). This leads to very
> nasty problems that people nowdays forget about.
> </OldFart>

Not just K&R.  These are different because of varargs:

	printf ("%p", NULL);
	printf ("%p", 0);

-- Jamie

NULL. Again. (was Re: [PATCH] 2.4.22pre10: {,un}likely_p())

[Chip Salzenberg]

According to Jamie Lokier:
> Not just K&R.  These are different because of varargs:
> 	printf ("%p", NULL);
> 	printf ("%p", 0);

*SIGH*  I thought incorrect folk wisdom about NULL and zero and pointer
conversions had long since died out.  More fool I.  Please, *please*,
_no_one_else_ argue about NULL/zero/false etc. until after reading this:

  ===[[  http://www.eskimo.com/~scs/C-faq/s5.html  ]]===

I thank you, and linux users everywhere thank you.
-- 
Chip Salzenberg               - a.k.a. -               <chip@pobox.com>
"I wanted to play hopscotch with the impenetrable mystery of existence,
    but he stepped in a wormhole and had to go in early."  // MST3K

Re: NULL. Again. (was Re: [PATCH] 2.4.22pre10: {,un}likely_p())

[Sean Neakums]

Chip Salzenberg <chip@pobox.com> writes:

> According to Jamie Lokier:
>> Not just K&R.  These are different because of varargs:
>> 	printf ("%p", NULL);
>> 	printf ("%p", 0);
>
> *SIGH*  I thought incorrect folk wisdom about NULL and zero and pointer
> conversions had long since died out.  More fool I.  Please, *please*,
> _no_one_else_ argue about NULL/zero/false etc. until after reading this:
>
>   ===[[  http://www.eskimo.com/~scs/C-faq/s5.html  ]]===
>
> I thank you, and linux users everywhere thank you.

I had thought that the need for writing NULL where a pointer is
expected in varags functions was because the machine may have
different sizes for pointers and int.  In the case of the second
printf call above, if pointers are 64-bit and integers are 32-bit,
printf will read eight bytes from the stack, and only four will have
been occupied by the integer 0.

Re: NULL. Again. (was Re: [PATCH] 2.4.22pre10: {,un}likely_p())

[Jamie Lokier]

Chip Salzenberg wrote:
> According to Jamie Lokier:
> > Not just K&R.  These are different because of varargs:
> > 	printf ("%p", NULL);
> > 	printf ("%p", 0);
> 
> *SIGH*  I thought incorrect folk wisdom about NULL and zero and pointer
> conversions had long since died out.  More fool I.  Please, *please*,
> _no_one_else_ argue about NULL/zero/false etc. until after reading this:
> 
>   ===[[  http://www.eskimo.com/~scs/C-faq/s5.html  ]]===

Thanks Chip; I am much enlightened.  That is a fine URL.

To onlookers: neither of those printf statements is portable :)

-- Jamie

Re: NULL. Again. (was Re: [PATCH] 2.4.22pre10: {,un}likely_p())

[Jamie Lokier]

Sean Neakums wrote:
> I had thought that the need for writing NULL where a pointer is
> expected in varags functions was because the machine may have
> different sizes for pointers and int.  In the case of the second
> printf call above, if pointers are 64-bit and integers are 32-bit,
> printf will read eight bytes from the stack, and only four will have
> been occupied by the integer 0.

It is incorrect to write NULL as a varargs argument.  It won't
necessarily pass a pointer because:

   1. a valid definition of NULL is "0".

If you want to pass a pointer to varargs, you might have considered
writing "(void *) 0", but that doesn't work because:

   2. a machine may have different sizes for different pointer types.

You must write "(type) 0" or "(type) NULL", using the correct pointer type.

-- Jamie

Re: NULL. Again. (was Re: [PATCH] 2.4.22pre10: {,un}likely_p())

[Timothy Miller]

Sean Neakums wrote:
> Chip Salzenberg <chip@pobox.com> writes:
> 
> 
>>According to Jamie Lokier:
>>
>>>Not just K&R.  These are different because of varargs:
>>>	printf ("%p", NULL);
>>>	printf ("%p", 0);
>>
>>*SIGH*  I thought incorrect folk wisdom about NULL and zero and pointer
>>conversions had long since died out.  More fool I.  Please, *please*,
>>_no_one_else_ argue about NULL/zero/false etc. until after reading this:
>>
>>  ===[[  http://www.eskimo.com/~scs/C-faq/s5.html  ]]===
>>
>>I thank you, and linux users everywhere thank you.
> 
> 
> I had thought that the need for writing NULL where a pointer is
> expected in varags functions was because the machine may have
> different sizes for pointers and int.  In the case of the second
> printf call above, if pointers are 64-bit and integers are 32-bit,
> printf will read eight bytes from the stack, and only four will have
> been occupied by the integer 0.
> 

Yes, but you're leaving information out.  If you read the FAQ, you'll 
find that NULL would not be the right thing to use in some cases.  For 
instance, (char *)0 may be a different value from (void *)0, so it's 
best to cast the pointer when passing to printf or something which uses 
varargs.