powerpc: Don't use long for 32-bit temp variables in spin lock ops
PATCH 1/1 powerpc: Don't use long for 32-bit temp variables in spin lock ops
The spinlock ops have long as their return type (as well as for some of the temporary types internally). All locks are 32-bit, so it makes no sense to do 64-bit ops on them.
For example, this is how my compiler built _spin_lock() for me:
c0000000004b2050: li r0,0 c0000000004b2054: stb r0,460(r13) c0000000004b2058: lwz r0,8(r13) c0000000004b205c: lwarx r9,0,r3 c0000000004b2060: cmpwi r9,0 c0000000004b2064: bne- c0000000004b2078 <._spin_lock+0x28> c0000000004b2068: stwcx. r0,0,r3 c0000000004b206c: nop c0000000004b2070: bne+ c0000000004b205c <._spin_lock+0xc> c0000000004b2074: isync c0000000004b2078: cmpdi cr7,r9,0 c0000000004b207c: .long 0x4dfe0020 c0000000004b2080: mr r1,r1 c0000000004b2084: lwz r0,0(r3) c0000000004b2088: cmpdi cr7,r0,0 c0000000004b208c: bne+ cr7,c0000000004b2080 <._spin_lock+0x30> c0000000004b2090: mr r2,r2 c0000000004b2094: b c0000000004b2058 <._spin_lock+0x8>
Note the cmpdi at ..78 when r9 was loaded with lwarx.
Unfortunately I haven't found a way to get rid of the duplicate comparison alltogether.
include/asm-powerpc/spinlock.h +7 -7
@@ -53,9 +53,9 @@* This returns the old value in the lock, so we succeeded * in getting the lock if the return value is 0. */ -static __inline__unsigned long __spin_trylock (raw_spinlock_t *lock) +static __inline__unsigned int __spin_trylock (raw_spinlock_t *lock) { -unsigned long tmp, token; +unsigned int tmp, token; token =LOCK_TOKEN ; __asm__ __volatile__(
@@ -179,9 +179,9 @@ extern void __raw_spin_unlock_wait(raw_s* This returns the old value in the lock + 1, * so we got a read lock if the return value is > 0. */ -static long __inline____read_trylock (raw_rwlock_t *rw) +static int __inline____read_trylock (raw_rwlock_t *rw) { -long tmp; +int tmp; __asm__ __volatile__( "1: lwarx %0,0,%1\n"
@@ -203,9 +203,9 @@ static long __inline__ __read_trylock(ra* This returns the old value in the lock, * so we got the write lock if the return value is 0. */ -static __inline__long __write_trylock (raw_rwlock_t *rw) +static __inline__int __write_trylock (raw_rwlock_t *rw) { -long tmp, token; +int tmp, token; token =WRLOCK_TOKEN ; __asm__ __volatile__(
@@ -263,7 +263,7 @@ static int __inline__ __raw_write_trylocstatic void __inline____raw_read_unlock (raw_rwlock_t *rw) { -long tmp; +int tmp; __asm__ __volatile__( "# read_unlock\n\t"
Quoted 25 lines · Olof Johansson
The spinlock ops have long as their return type (as well as for some of the temporary types internally). All locks are 32-bit, so it makes no sense to do 64-bit ops on them.
For example, this is how my compiler built
_spin_lock()for me:c0000000004b2050: li r0,0 c0000000004b2054: stb r0,460(r13) c0000000004b2058: lwz r0,8(r13) c0000000004b205c: lwarx r9,0,r3 c0000000004b2060: cmpwi r9,0 c0000000004b2064: bne- c0000000004b2078 <._spin_lock+0x28> c0000000004b2068: stwcx. r0,0,r3 c0000000004b206c: nop c0000000004b2070: bne+ c0000000004b205c <._spin_lock+0xc> c0000000004b2074: isync c0000000004b2078: cmpdi cr7,r9,0 c0000000004b207c: .long 0x4dfe0020 c0000000004b2080: mr r1,r1 c0000000004b2084: lwz r0,0(r3) c0000000004b2088: cmpdi cr7,r0,0 c0000000004b208c: bne+ cr7,c0000000004b2080 <._spin_lock+0x30> c0000000004b2090: mr r2,r2 c0000000004b2094: b c0000000004b2058 <._spin_lock+0x8>
Quoted 1 line
Note the cmpdi at ..78 when r9 was loaded with lwarx.
That's OK; the lwarx is defined to zero-extend the value to 64 bits.
Or are you implying that cmpdi is slower than cmpwi on some 64-bit processors? :)
Paul.
Quoted 6 lines · Paul Mackerras
Quoted 1 line
Note the cmpdi at ..78 when r9 was loaded with lwarx.
That's OK; the lwarx is defined to zero-extend the value to 64 bits.
Or are you implying that cmpdi is slower than cmpwi on some 64-bit processors? :)
Nope, it was just seemed inconsistent to me, especially since the preceding (asm) comparison was 32-bit.
-Olof