Skip to contentSkip to conversation

gcc-3.1 ffs problem, kernel 2.4.18

PATCH In review Michal Moskal 3 participants 5 messages Mar 20, 2002 – Apr 3, 2002

lore
Share & tools
lore

Michal Moskal

author
#0 lore raw

PATCH 1/1 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.

include/asm-i386/bitops.h +25 -11
diff -ur linux-/include/asm-i386/bitops.h linux/include/asm-i386/bitops.h
@@ -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;
+	}
 }
 
 /**
Signature · 2 lines
: Michal Moskal :::::::: malekith/at/pld.org.pl :  GCS {C,UL}++++$ a? !tv
: PLD Linux ::::::: Wroclaw University, CS Dept :  {E-,w}-- {b++,e}>+++ h
#1 lore raw
Quoted 2 lines · Michal Moskal

Following patch is needed to compile linux-2.4.18 with xfs patch with recent gcc 3.1 snapshot.

What is the failure mode?

Quoted 2 lines

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.

Quoted 2 lines

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

Thanks for testing though.

r~

#2 lore raw
In reply to Michal Moskal
Quoted 1 line · Michal Moskal

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

Quoted 4 lines
	__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~

#3 lore raw
Quoted 5 lines · Richard Henderson

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

#4 lore raw
In reply to Richard Henderson
Quoted 2 lines · Linus Torvalds

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~