2002-03-20 17:43:15

by Michal Moskal

[permalink] [raw]
Subject: [PATCH] gcc-3.1 ffs problem, kernel 2.4.18

Following patch is needed to compile linux-2.4.18 with xfs patch with
recent gcc 3.1 snapshot. It is clearly bug in linux includes (they use
asm code unconditionally, even on constants, and they can get constants
if exposed to tree-inliner). Other asm includes might need fixing too,
I saw asm-cris is fixed already.

Beside this linux-2.4.18 compiled with gcc-3.1 20020311 works fine for
me.

diff -ur linux-/include/asm-i386/bitops.h linux/include/asm-i386/bitops.h
--- linux-/include/asm-i386/bitops.h Thu Nov 22 20:46:18 2001
+++ linux/include/asm-i386/bitops.h Tue Mar 5 21:08:15 2002
@@ -316,6 +316,8 @@
return (offset + set + res);
}

+#include <linux/bitops.h>
+
/**
* ffz - find first zero in word.
* @word: The word to search
@@ -324,10 +326,16 @@
*/
static __inline__ unsigned long ffz(unsigned long word)
{
- __asm__("bsfl %1,%0"
- :"=r" (word)
- :"r" (~word));
- return word;
+ /* The generic_ffs function is used to avoid the asm when the
+ argument is a constant. */
+ if (__builtin_constant_p (word)) {
+ return (~word ? (unsigned long) generic_ffs ((int) ~word) - 1 : 32);
+ } else {
+ __asm__("bsfl %1,%0"
+ :"=r" (word)
+ :"r" (~word));
+ return word;
+ }
}

#ifdef __KERNEL__
@@ -342,13 +350,19 @@
*/
static __inline__ int ffs(int x)
{
- int r;
-
- __asm__("bsfl %1,%0\n\t"
- "jnz 1f\n\t"
- "movl $-1,%0\n"
- "1:" : "=r" (r) : "g" (x));
- return r+1;
+ /* The generic_ffs function is used to avoid the asm when the
+ argument is a constant. */
+ if (__builtin_constant_p (x)) {
+ return generic_ffs (x);
+ } else {
+ int r;
+
+ __asm__("bsfl %1,%0\n\t"
+ "jnz 1f\n\t"
+ "movl $-1,%0\n"
+ "1:" : "=r" (r) : "g" (x));
+ return r+1;
+ }
}

/**

--
: Michal Moskal :::::::: malekith/at/pld.org.pl : GCS {C,UL}++++$ a? !tv
: PLD Linux ::::::: Wroclaw University, CS Dept : {E-,w}-- {b++,e}>+++ h


2002-03-29 05:10:52

by Richard Henderson

[permalink] [raw]
Subject: Re: [PATCH] gcc-3.1 ffs problem, kernel 2.4.18

On Wed, Mar 20, 2002 at 06:42:38PM +0100, Michal Moskal wrote:
> Following patch is needed to compile linux-2.4.18 with xfs patch with
> recent gcc 3.1 snapshot.

What is the failure mode?

> It is clearly bug in linux includes (they use asm code unconditionally,
> even on constants, ...

So? I don't see that that's a *bug* at all. A missed optimization
opportunity yes, bug no.


> Beside this linux-2.4.18 compiled with gcc-3.1 20020311 works fine for
> me.

Thanks for testing though.


r~

2002-04-02 23:00:16

by Richard Henderson

[permalink] [raw]
Subject: Re: [PATCH] gcc-3.1 ffs problem, kernel 2.4.18

On Tue, Apr 02, 2002 at 01:48:48PM +0200, Michal Moskal wrote:
> Could you please check if it compiles for you?

Ah, I see the bug. I was looking at ffz (which is correct),
not ffs (which isn't).

> __asm__("bsfl %1,%0\n\t"
> "jnz 1f\n\t"
> "movl $-1,%0\n"
> "1:" : "=r" (r) : "g" (x));

The problem is ^^^

That sez any of register, memory, or immediate is ok.
It should be "r" instead, just like in ffz.

That said, we should probably be using __builtin_ffs
instead. The compiler knows how to do bsfl plus the
adjustment. Plus, it knows how to evaluate it at
compile-time for constants.


r~

2002-04-03 00:05:44

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] gcc-3.1 ffs problem, kernel 2.4.18


On Tue, 2 Apr 2002, Richard Henderson wrote:
>
> That said, we should probably be using __builtin_ffs
> instead. The compiler knows how to do bsfl plus the
> adjustment. Plus, it knows how to evaluate it at
> compile-time for constants.

When was __builtin_ffs introduced? I know it didn't use to exist, but
we've obviously bumped up the gcc requirements several times, so..

Linus

2002-04-03 11:00:36

by Richard Henderson

[permalink] [raw]
Subject: Re: [PATCH] gcc-3.1 ffs problem, kernel 2.4.18

On Tue, Apr 02, 2002 at 04:04:58PM -0800, Linus Torvalds wrote:
> When was __builtin_ffs introduced? I know it didn't use to exist, but
> we've obviously bumped up the gcc requirements several times, so..

Dunno. Before egcs forked.


r~