linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] 2.4.22pre10: {,un}likely_p() macros for pointers
@ 2003-08-10  4:03 Albert Cahalan
  2003-08-10  7:29 ` Willy Tarreau
  0 siblings, 1 reply; 26+ messages in thread
From: Albert Cahalan @ 2003-08-10  4:03 UTC (permalink / raw)
  To: linux-kernel mailing list; +Cc: willy, davem, jamie, chip, jamie, albert

Willy Tarreau writes:
>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).

I looked at the assembly (ppc, gcc 3.2.3) and didn't
see any overhead.

Note that the kernel code is broken for the likely()
macro, since 42 is a perfectly good truth value in C.

> 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.

The !!x gives you a 0 or 1 while converting the type.
So a (char*)0xfda9c80300000000ull will become a 1 of
an acceptable type, allowing the macro to work as desired.



^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH] 2.4.22pre10: {,un}likely_p() macros for pointers
@ 2003-08-05 12:44 Albert Cahalan
  2003-08-09  0:21 ` Jamie Lokier
  0 siblings, 1 reply; 26+ messages in thread
From: Albert Cahalan @ 2003-08-05 12:44 UTC (permalink / raw)
  To: linux-kernel mailing list, chip

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; \
})



^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH] 2.4.22pre10: {,un}likely_p() macros for pointers
@ 2003-08-04 17:06 Chip Salzenberg
  0 siblings, 0 replies; 26+ messages in thread
From: Chip Salzenberg @ 2003-08-04 17:06 UTC (permalink / raw)
  To: Linux Kernel

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.

PS: This patch was made against 2.4.22pre10 plus many patches from the
'aa' kernel series, so it should be considered an example of the patch
that might be required in other kernel versions.

Index: linux/include/linux/compiler.h
--- linux/include/linux/compiler.h.old	2001-09-18 17:12:45.000000000 -0400
+++ linux/include/linux/compiler.h	2003-08-04 12:24:15.000000000 -0400
@@ -11,6 +11,8 @@
 #endif
 
-#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)
 
 #endif /* __LINUX_COMPILER_H */

Index: linux/kernel/sched.c
--- linux/kernel/sched.c.old	2003-08-04 12:09:47.000000000 -0400
+++ linux/kernel/sched.c	2003-08-04 12:25:44.000000000 -0400
@@ -477,5 +477,5 @@
 	if (unlikely(!prev->mm)) {
 		prev->active_mm = NULL;
-		if (unlikely(rq->prev_mm)) {
+		if (unlikely_p(rq->prev_mm)) {
 			printk(KERN_ERR "rq->prev_mm was %p set to %p - %s\n", rq->prev_mm, oldmm, current->comm);
 			dump_stack();

Index: linux/mm/filemap.c
--- linux/mm/filemap.c.old	2003-08-04 12:09:41.000000000 -0400
+++ linux/mm/filemap.c	2003-08-04 12:27:07.000000000 -0400
@@ -3749,5 +3749,5 @@
 		pr_debug("attempting to read %lu\n", page->index);
 		io->did_read = 1;
-		if (likely(page->mapping)) {
+		if (likely_p(page->mapping)) {
 			locked = 0;
 			io->err = page->mapping->a_ops->readpage(io->file, page);
@@ -3813,5 +3813,5 @@
 		 */
 		if (!TryLockPage(page)) {
-			if (likely(page->mapping)) {
+			if (likely_p(page->mapping)) {
 				int ret = readpage(io->file, page);
 				if (ret)



-- 
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

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2003-08-13 19:43 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-08-10  4:03 [PATCH] 2.4.22pre10: {,un}likely_p() macros for pointers Albert Cahalan
2003-08-10  7:29 ` Willy Tarreau
2003-08-10  8:02   ` Willy Tarreau
2003-08-11  1:23   ` Chip Salzenberg
2003-08-11  2:09     ` Jamie Lokier
2003-08-11  2:39       ` Chip Salzenberg
2003-08-11  4:02         ` Albert Cahalan
2003-08-11  4:30         ` Jamie Lokier
2003-08-11  5:30         ` Willy Tarreau
2003-08-11  5:42           ` Jamie Lokier
2003-08-11 13:09             ` Albert Cahalan
2003-08-11 18:55               ` Andrew Morton
2003-08-11 23:13                 ` Albert Cahalan
2003-08-13 19:42                   ` Jamie Lokier
2003-08-11  4:55   ` Jamie Lokier
2003-08-11  5:26     ` Willy Tarreau
2003-08-11  5:38       ` Jamie Lokier
  -- strict thread matches above, loose matches on Subject: below --
2003-08-05 12:44 Albert Cahalan
2003-08-09  0:21 ` Jamie Lokier
2003-08-09  8:13   ` Willy Tarreau
2003-08-09  8:51     ` David S. Miller
2003-08-09  9:36       ` Jamie Lokier
2003-08-09 10:10       ` Herbert Xu
2003-08-09 10:42       ` Alan Cox
2003-08-09 16:23         ` Jamie Lokier
2003-08-04 17:06 Chip Salzenberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).