Re: [RFC] x86: bitops asm constraint fixes
From: Jan Beulich
Date: Mon Apr 14 2008 - 08:53:18 EST
>>> Ingo Molnar <mingo@xxxxxxx> 27.03.08 09:41 >>>
>
>* Jan Beulich <jbeulich@xxxxxxxxxx> wrote:
>
>> Please revert it for the time being, I've got a better version (i.e.
>> without extra dead code being generated) that I intended to submit
>> once I know whether the other issues pointed out in the description on
>> the original patch also should be adjusted. Of course, that could also
>> be done incrementally, but I would think overhauling the whole file at
>> once wouldn't be a bad thing...
>
>since it appears to cause no problems in x86.git (it passed a lot of
>testing already) i'd prefer to keep it (so that we can see any other
>side-effects of touching this code) - could you send your improvements
>as a delta against x86.git/latest? [or is there any outright bug caused
>by your changes that necessiates a revert?]
Finally got around to it:
For those bitops that don't have a memory clobber, make the asm
constraints more precise. It still remains questionable whether the
inconsistencies in the use of memory clobbers shouldn't be addressed.
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
--- x86.git/include/asm-x86/bitops.h
+++ x86.git/include/asm-x86/bitops.h
@@ -20,16 +20,20 @@
* bit 0 is the LSB of addr; bit 32 is the LSB of (addr+1).
*/
+struct __bits { int _[1UL << (32 - 3)]; };
+
#if __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 1)
/* Technically wrong, but this avoids compilation errors on some gcc
versions. */
-#define ADDR "=m" (*(volatile long *)addr)
-#define BIT_ADDR "=m" (((volatile int *)addr)[nr >> 5])
+#define ADDR "=m" (*(volatile long *) addr)
+#define BIT_ADDR "=m" (((volatile int *) addr)[nr >> 5])
+#define FULL_ADDR "=m" (*(volatile struct __bits *) addr)
#else
#define ADDR "+m" (*(volatile long *) addr)
-#define BIT_ADDR "+m" (((volatile int *)addr)[nr >> 5])
+#define BIT_ADDR "+m" (((volatile int *) addr)[nr >> 5])
+#define FULL_ADDR "+m" (*(volatile struct __bits *) addr)
#endif
-#define BASE_ADDR "m" (*(volatile int *)addr)
+#define BASE_ADDR "m" (*(volatile int *) addr)
/**
* set_bit - Atomically set a bit in memory
@@ -62,7 +68,10 @@ static inline void set_bit(int nr, volat
*/
static inline void __set_bit(int nr, volatile void *addr)
{
- asm volatile("bts %1,%0" : ADDR : "Ir" (nr) : "memory");
+ if (__builtin_constant_p(nr))
+ asm volatile("bts %1,%2" : BIT_ADDR : "Ir" (nr), BASE_ADDR);
+ else
+ asm volatile("bts %1,%0" : FULL_ADDR : "r" (nr));
}
/**
@@ -77,7 +87,11 @@ static inline void __set_bit(int nr, vol
*/
static inline void clear_bit(int nr, volatile void *addr)
{
- asm volatile(LOCK_PREFIX "btr %1,%2" : BIT_ADDR : "Ir" (nr), BASE_ADDR);
+ if (__builtin_constant_p(nr))
+ asm volatile(LOCK_PREFIX "btr %1,%2"
+ : BIT_ADDR : "Ir" (nr), BASE_ADDR);
+ else
+ asm volatile(LOCK_PREFIX "btr %1,%0" : FULL_ADDR : "r" (nr));
}
/*
@@ -96,7 +110,10 @@ static inline void clear_bit_unlock(unsi
static inline void __clear_bit(int nr, volatile void *addr)
{
- asm volatile("btr %1,%2" : BIT_ADDR : "Ir" (nr), BASE_ADDR);
+ if (__builtin_constant_p(nr))
+ asm volatile("btr %1,%2" : BIT_ADDR : "Ir" (nr), BASE_ADDR);
+ else
+ asm volatile("btr %1,%0" : FULL_ADDR : "r" (nr));
}
/*
@@ -131,7 +148,10 @@ static inline void __clear_bit_unlock(un
*/
static inline void __change_bit(int nr, volatile void *addr)
{
- asm volatile("btc %1,%2" : BIT_ADDR : "Ir" (nr), BASE_ADDR);
+ if (__builtin_constant_p(nr))
+ asm volatile("btc %1,%2" : BIT_ADDR : "Ir" (nr), BASE_ADDR);
+ else
+ asm volatile("btc %1,%0" : FULL_ADDR : "r" (nr));
}
/**
@@ -145,7 +165,11 @@ static inline void __change_bit(int nr,
*/
static inline void change_bit(int nr, volatile void *addr)
{
- asm volatile(LOCK_PREFIX "btc %1,%2" : BIT_ADDR : "Ir" (nr), BASE_ADDR);
+ if (__builtin_constant_p(nr))
+ asm volatile(LOCK_PREFIX "btc %1,%2"
+ : BIT_ADDR : "Ir" (nr), BASE_ADDR);
+ else
+ asm volatile(LOCK_PREFIX "btc %1,%0" : FULL_ADDR : "r" (nr));
}
/**
@@ -191,9 +217,15 @@ static inline int __test_and_set_bit(int
{
int oldbit;
- asm volatile("bts %2,%3\n\t"
- "sbb %0,%0"
- : "=r" (oldbit), BIT_ADDR : "Ir" (nr), BASE_ADDR);
+ if (__builtin_constant_p(nr))
+ asm volatile("bts %2,%3\n\t"
+ "sbb %0,%0"
+ : "=r" (oldbit), BIT_ADDR : "Ir" (nr), BASE_ADDR);
+ else
+ asm volatile("bts %2,%1\n\t"
+ "sbb %0,%0"
+ : "=r" (oldbit), FULL_ADDR : "r" (nr));
+
return oldbit;
}
@@ -229,9 +262,15 @@ static inline int __test_and_clear_bit(i
{
int oldbit;
- asm volatile("btr %2,%3\n\t"
- "sbb %0,%0"
- : "=r" (oldbit), BIT_ADDR : "Ir" (nr), BASE_ADDR);
+ if (__builtin_constant_p(nr))
+ asm volatile("btr %2,%3\n\t"
+ "sbb %0,%0"
+ : "=r" (oldbit), BIT_ADDR : "Ir" (nr), BASE_ADDR);
+ else
+ asm volatile("btr %2,%1\n\t"
+ "sbb %0,%0"
+ : "=r" (oldbit), FULL_ADDR : "r" (nr));
+
return oldbit;
}
@@ -240,9 +279,14 @@ static inline int __test_and_change_bit(
{
int oldbit;
- asm volatile("btc %2,%3\n\t"
- "sbb %0,%0"
- : "=r" (oldbit), BIT_ADDR : "Ir" (nr), BASE_ADDR);
+ if (__builtin_constant_p(nr))
+ asm volatile("btc %2,%3\n\t"
+ "sbb %0,%0"
+ : "=r" (oldbit), BIT_ADDR : "Ir" (nr), BASE_ADDR);
+ else
+ asm volatile("btc %2,%1\n\t"
+ "sbb %0,%0"
+ : "=r" (oldbit), FULL_ADDR : "r" (nr));
return oldbit;
}
@@ -276,11 +321,10 @@ static inline int variable_test_bit(int
{
int oldbit;
- asm volatile("bt %2,%3\n\t"
+ asm volatile("bt %2,%1\n\t"
"sbb %0,%0"
: "=r" (oldbit)
- : "m" (((volatile const int *)addr)[nr >> 5]),
- "Ir" (nr), BASE_ADDR);
+ : "m" (*(volatile struct __bits *) addr), "Ir" (nr));
return oldbit;
}
@@ -398,6 +431,7 @@ static int test_bit(int nr, const volati
#endif /* __KERNEL__ */
#undef BASE_ADDR
+#undef FULL_ADDR
#undef BIT_ADDR
#undef ADDR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/