2002-04-23 17:28:14

by Keith Owens

[permalink] [raw]
Subject: [patch] 2.5.9 remove warnings

exit.c:40: warning: passing arg 1 of `__builtin_expect' makes integer from pointer without a cast

Index: 9.2/kernel/exit.c
--- 9.2/kernel/exit.c Tue, 23 Apr 2002 11:21:19 +1000 kaos (linux-2.5/w/d/25_exit.c 1.11.1.4 644)
+++ 9.2(w)/kernel/exit.c Tue, 23 Apr 2002 13:37:06 +1000 kaos (linux-2.5/w/d/25_exit.c 1.11.1.4 644)
@@ -37,7 +37,7 @@ static inline void __unhash_process(stru
list_del(&p->thread_group);
p->pid = 0;
proc_dentry = p->proc_dentry;
- if (unlikely(proc_dentry)) {
+ if (unlikely(proc_dentry != NULL)) {
spin_lock(&dcache_lock);
if (!list_empty(&proc_dentry->d_hash)) {
dget_locked(proc_dentry);
@@ -47,7 +47,7 @@ static inline void __unhash_process(stru
spin_unlock(&dcache_lock);
}
write_unlock_irq(&tasklist_lock);
- if (unlikely(proc_dentry)) {
+ if (unlikely(proc_dentry != NULL)) {
shrink_dcache_parent(proc_dentry);
dput(proc_dentry);
}


2002-04-23 18:31:58

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch] 2.5.9 remove warnings

Keith Owens wrote:
>
> ...
> @@ -47,7 +47,7 @@ static inline void __unhash_process(stru
> spin_unlock(&dcache_lock);
> }
> write_unlock_irq(&tasklist_lock);
> - if (unlikely(proc_dentry)) {
> + if (unlikely(proc_dentry != NULL)) {
> shrink_dcache_parent(proc_dentry);
> dput(proc_dentry);
> }

Is it not possible to fix it for all time?

--- linux-2.5.9/include/linux/compiler.h Sun Apr 14 15:45:08 2002
+++ 25/include/linux/compiler.h Tue Apr 23 11:27:37 2002
@@ -10,8 +10,8 @@
#define __builtin_expect(x, expected_value) (x)
#endif

-#define likely(x) __builtin_expect((x),1)
-#define unlikely(x) __builtin_expect((x),0)
+#define likely(x) __builtin_expect((x) != 0, 1)
+#define unlikely(x) __builtin_expect((x) != 0, 0)

/* This macro obfuscates arithmetic on a variable address so that gcc
shouldn't recognize the original var, and make assumptions about it */

(Interestingly, this patch shrinks my kernel by 32 bytes. hmm.)

BTW, it would be very useful if someone could invert the sense of
`likely' and `unlikely', so they always say the *wrong* thing, and
then actually demonstrate some real-world slowdown. coz if this
can't be done, why are we putting up with the visual clutter?

-

2002-04-23 21:08:14

by Kasper Dupont

[permalink] [raw]
Subject: Re: [patch] 2.5.9 remove warnings

Andrew Morton wrote:
>
> Is it not possible to fix it for all time?
>
> --- linux-2.5.9/include/linux/compiler.h Sun Apr 14 15:45:08 2002
> +++ 25/include/linux/compiler.h Tue Apr 23 11:27:37 2002
> @@ -10,8 +10,8 @@
> #define __builtin_expect(x, expected_value) (x)
> #endif
>
> -#define likely(x) __builtin_expect((x),1)
> -#define unlikely(x) __builtin_expect((x),0)
> +#define likely(x) __builtin_expect((x) != 0, 1)
> +#define unlikely(x) __builtin_expect((x) != 0, 0)
>
> /* This macro obfuscates arithmetic on a variable address so that gcc
> shouldn't recognize the original var, and make assumptions about it */

Wouldn't "!!(x)" make more sense here than "(x) != 0"?
(I don't like comparing pointers with integers.)

--
Kasper Dupont -- der bruger for meget tid p? usenet.
For sending spam use mailto:[email protected]

2002-04-24 09:37:03

by Daniel Phillips

[permalink] [raw]
Subject: Re: [patch] 2.5.9 remove warnings

On Tuesday 23 April 2002 05:39, Keith Owens wrote:
> exit.c:40: warning: passing arg 1 of `__builtin_expect' makes integer from pointer without a cast

> - if (unlikely(proc_dentry)) {
> + if (unlikely(proc_dentry != NULL)) {

Have you checked to see if this produces the same code? To be on the safe
side:

+ if (unlikely((ulong) proc_dentry)) {

--
Daniel

2002-04-24 18:57:30

by Ingo Oeser

[permalink] [raw]
Subject: Re: [patch] 2.5.9 remove warnings

On Tue, Apr 23, 2002 at 11:08:08PM +0200, Kasper Dupont wrote:
> Wouldn't "!!(x)" make more sense here than "(x) != 0"?
> (I don't like comparing pointers with integers.)

No, == 0 means, that the pointer is invalid, so != 0 means, the
pointer is valid. You can find this in the C99-Standard at least.

So, this is perfectly normal ;-)

Regards

Ingo Oeser
--
Science is what we can tell a computer. Art is everything else. --- D.E.Knuth