HPPA Linuxthreads.
PATCH 1/1 HPPA Linuxthreads.
libc-alpha,
This is a cleaner HPPA linuxthreads implementation that stems from the work that John David Anglin <[email protected]> and myself did to devise a self-aligning lock system that doesn't impose the 16-byte lock alignment restriction. Many thanks go to John for all his hard work!
This will also allow hppa to relax malloc alignment back to 8 (future patches).
libc/linuxthreads/*
sysdeps/hppa/pt-machine.h | 81 ++++++++++++++++++++++----- sysdeps/hppa/pspinlock.c | 29 ++++----- sysdeps/unix/sysv/linux/hppa/bits/initspin.h | 22 +++++--
sysdeps/pthread/bits/libc-lock.h | 6 +- sysdeps/pthread/bits/pthreadtypes.h | 8 +-
descr.h | 2 pt-machine.c | 4 + pthread.c | 14 ++-- spinlock.c | 24 ++++---- spinlock.h | 26 ++++++--
10 files changed, 148 insertions(+), 68 deletions(-)
Comments welcome. Patches tested on HPPA, i686.
c.
---
2003-05-19 Carlos O'Donell <[email protected]>
* linuxthreads/sysdeps/hppa/pt-machine.h (THREAD_SELF): Define.
(INIT_THREAD_SELF): Define.
(testandset): Use __ldcw_align.
(lock_held): Define.
(__ldcw): Define.
(__ldcw_align): Define.
(__load_and_clear): Define.
* linuxthreads/sysdeps/hppa/pspinlock.c (__pthread_spin_lock): Use __ldcw_align. (__pthread_spin_trylock): Likewise. (__pthread_spin_unlock): Likewise. (__pthread_spin_init): Likewise.
* linuxthreads/sysdeps/unix/sysv/linux/hppa/bits/initspin.h (__LT_SPINLOCK_INIT): Define. (__LT_SPINLOCK_ALT_INIT): Define. (__LOCK_INITIALIZER): Define. (__LOCK_ALT_INITIALIZER): Define. (__ATOMIC_INITIALIZER): Define. (__LT_INITIALIZER_NOT_ZERO): Define.
* linuxthreads/sysdeps/pthread/bits/libc-lock.h:
Use __LT_INITIALIZER_NOT_ZERO instead of __LT_SPINLOCK_INIT.
* linuxthreads/sysdeps/pthread/bits/pthreadtypes.h:
Add global default definition for __atomic_lock_t. (pthread_fastlock): Change __spinlock to type __atomic_lock_t.
* linuxthreads/descr.h (pthread_atomic): change p_spinlock to type __atomic_lock_t from int *.
* linuxthreads/pt-machine.c: include pthread.h, change extern testandset prototype to use __atomic_lock_t * spinlock.
* linuxthreads/pthread.c (__pthread_initialize_minimal):
Use __LT_INITIALIZER_NOT_ZERO.
* linuxthreads/spinlock.c: use __pthread_lock_define_initialized macro to initialize wait_node_free_list_spinlock. (__pthread_acquire): change prototype to use __atomic_lock_t * spinlock.
(__pthread_release): Likewise. (__pthread_compare_and_swap): Likewise. (__pthread_acquire): Likewise.
(__pthread_alt_lock): Use __LT_SPINLOCK_INIT to initialize locks. (__pthread_alt_timedlock): Likewise.
* linuxthreads/spinlock.h: define default lock_held, __pthread_lock_define_initialized, and modify prototypes for __pthread_compare_and_swap, compare_and_swap, compare_and_swap_with_release_semantics, compare_and_swap, __pthread_compare_and_swap to use __atomic_lock_t * spinlock.
linuxthreads/descr.h +1 -1
@@ -70,7 +70,7 @@/* Atomic counter made possible by compare_and_swap */ struct pthread_atomic {long p_count; -int p_spinlock; +__atomic_lock_t p_spinlock; };
linuxthreads/pt-machine.c +3 -1
@@ -19,7 +19,9 @@#define PT_EI -extern long int testandset (int *spinlock); +#include <pthread.h> + +extern long int testandset (__atomic_lock_t *spinlock);extern int __compare_and_swap (long int *p,long int oldval,long int newval);#include <pt-machine.h>
linuxthreads/pthread.c +7 -7
@@ -296,9 +296,9 @@pthread_descr self;/* First of all init __pthread_handles[0] and [1] if needed. */ -# if __LT_SPINLOCK_INIT !=0 - __pthread_handles[0 ].h_lock = __LOCK_INITIALIZER; - __pthread_handles[1 ].h_lock = __LOCK_INITIALIZER; +# ifdef __LT_INITIALIZER_NOT_ZERO + __pthread_handles[0 ].h_lock = __LOCK_ALT_INITIALIZER; + __pthread_handles[1 ].h_lock = __LOCK_ALT_INITIALIZER;# endif # ifndef SHARED /* Unlike in the dynamically linked case the dynamic linker has not
@@ -366,7 +366,7 @@# endif /* self->p_start_args need not be initialized, it's all zero. */ self->p_userstack =1 ; -# if __LT_SPINLOCK_INIT !=0 +# ifdef __LT_INITIALIZER_NOT_ZERO self->p_resume_count = (struct pthread_atomic ) __ATOMIC_INITIALIZER;# endif self->p_alloca_cutoff = __MAX_ALLOCA_CUTOFF;
@@ -380,9 +380,9 @@#else /* USE_TLS */ /* First of all init __pthread_handles[0] and [1]. */ -# if __LT_SPINLOCK_INIT !=0 - __pthread_handles[0 ].h_lock = __LOCK_INITIALIZER; - __pthread_handles[1 ].h_lock = __LOCK_INITIALIZER; +# ifdef __LT_INITIALIZER_NOT_ZERO + __pthread_handles[0 ].h_lock = __LOCK_ALT_INITIALIZER; + __pthread_handles[1 ].h_lock = __LOCK_ALT_INITIALIZER;# endif __pthread_handles[0 ].h_descr = &__pthread_initial_thread; __pthread_handles[1 ].h_descr = &__pthread_manager_thread;
linuxthreads/spinlock.c +11 -11
@@ -24,9 +24,9 @@#include "spinlock.h" #include "restart.h" -static void __pthread_acquire (int * spinlock); +static void __pthread_acquire (__atomic_lock_t * spinlock); -static inline void __pthread_release (int * spinlock) +static inline void __pthread_release (__atomic_lock_t * spinlock) {WRITE_MEMORY_BARRIER (); *spinlock = __LT_SPINLOCK_INIT;
@@ -269,11 +269,11 @@struct wait_node {struct wait_node *next;/* Next node in null terminated linked list */ pthread_descr thr;/* The thread waiting with this node */ -int abandoned;/* Atomic flag */ +__atomic_lock_t abandoned;/* Atomic flag */ };static long wait_node_free_list; -static int wait_node_free_list_spinlock; +__pthread_lock_define_initialized (static, wait_node_free_list_spinlock);/* Allocate a new node from the head of the free list using an atomic operation, or else using malloc if that list is empty. A fundamental
@@ -376,7 +376,7 @@if (self ==NULL ) self =thread_self (); - wait_node.abandoned =0 ; + wait_node.abandoned = __LT_SPINLOCK_INIT; wait_node.next = (struct wait_node *) lock->__status; wait_node.thr = self; lock->__status = (long ) &wait_node;
@@ -402,7 +402,7 @@ wait_node.thr = self; newstatus = (long ) &wait_node; } - wait_node.abandoned =0 ; + wait_node.abandoned = __LT_SPINLOCK_INIT; wait_node.next = (struct wait_node *) oldstatus;/* Make sure the store in wait_node.next completes before performing the compare-and-swap */
@@ -451,7 +451,7 @@if (self ==NULL ) self =thread_self (); - p_wait_node->abandoned =0 ; + p_wait_node->abandoned = __LT_SPINLOCK_INIT; p_wait_node->next = (struct wait_node *) lock->__status; p_wait_node->thr = self; lock->__status = (long ) p_wait_node;
@@ -474,7 +474,7 @@ p_wait_node->thr = self; newstatus = (long ) p_wait_node; } - p_wait_node->abandoned =0 ; + p_wait_node->abandoned = __LT_SPINLOCK_INIT; p_wait_node->next = (struct wait_node *) oldstatus;/* Make sure the store in wait_node.next completes before performing the compare-and-swap */
@@ -574,7 +574,7 @@while (p_node != (struct wait_node *)1 ) {int prio; -if (p_node->abandoned) { +if (lock_held (&p_node->abandoned)) {/* Remove abandoned node. */ #if definedTEST_FOR_COMPARE_AND_SWAP if (!__pthread_has_cas)
@@ -662,7 +662,7 @@#if !definedHAS_COMPARE_AND_SWAP || definedTEST_FOR_COMPARE_AND_SWAP int __pthread_compare_and_swap (long * ptr,long oldval,long newval, -int * spinlock) +__atomic_lock_t * spinlock) {int res;
@@ -699,7 +699,7 @@- When nanosleep() returns, we try again, doing MAX_SPIN_COUNT sched_yield(), then sleeping again if needed. */ -static void __pthread_acquire (int * spinlock) +static void __pthread_acquire (__atomic_lock_t * spinlock) {int cnt =0 ;struct timespec tm;
linuxthreads/spinlock.h +20 -6
@@ -33,14 +33,28 @@#endif #endif +/* Define lock_held for all arches that don't need a modified copy. */ +#ifndef __LT_INITIALIZER_NOT_ZERO +# define lock_held (p) *(p) +#endif + +/* Initliazers for possibly complex structures */ +#ifdef __LT_INITIALIZER_NOT_ZERO +# define __pthread_lock_define_initialized (CLASS ,NAME ) \ + CLASS __atomic_lock_t NAME = __LT_SPINLOCK_ALT_INIT +#else +# define __pthread_lock_define_initialized (CLASS ,NAME ) \ + CLASS __atomic_lock_t NAME +#endif +#if defined(TEST_FOR_COMPARE_AND_SWAP )extern int __pthread_has_cas;extern int __pthread_compare_and_swap (long * ptr,long oldval,long newval, -int * spinlock); +__atomic_lock_t * spinlock);static inline int compare_and_swap (long * ptr,long oldval,long newval, -int * spinlock) +__atomic_lock_t * spinlock) {if (__builtin_expect (__pthread_has_cas,1 ))return __compare_and_swap (ptr, oldval, newval);
@@ -58,7 +72,7 @@static inline int compare_and_swap_with_release_semantics (long * ptr,long oldval, -long newval,int * spinlock) +long newval,__atomic_lock_t * spinlock) {return __compare_and_swap_with_release_semantics (ptr, oldval, newval);
@@ -67,7 +81,7 @@#endif static inline int compare_and_swap (long * ptr,long oldval,long newval, -int * spinlock) +__atomic_lock_t * spinlock) {return __compare_and_swap (ptr, oldval, newval); }
@@ -75,10 +89,10 @@#else extern int __pthread_compare_and_swap (long * ptr,long oldval,long newval, -int * spinlock); +__atomic_lock_t * spinlock);static inline int compare_and_swap (long * ptr,long oldval,long newval, -int * spinlock) +__atomic_lock_t * spinlock) {return __pthread_compare_and_swap (ptr, oldval, newval, spinlock); }
linuxthreads/sysdeps/hppa/pspinlock.c +13 -16
@@ -24,15 +24,12 @@int __pthread_spin_lock (pthread_spinlock_t *lock) { -unsigned int val; +unsigned int *addr =__ldcw_align (lock); + +while (__ldcw (addr) ==0 ) +while (*addr ==0 ) ; -do - asmvolatile ("ldcw %1,%0" - :"=r" (val),"=m" (*lock) - :"m" (*lock)); -while (!val); - -return 0 ; +return 0 ; }weak_alias (__pthread_spin_lock, pthread_spin_lock)
@@ -40,13 +37,9 @@int __pthread_spin_trylock (pthread_spinlock_t *lock) { -unsigned int val; - - asmvolatile ("ldcw %1,%0" - :"=r" (val),"=m" (*lock) - :"m" (*lock)); +unsigned int *a =__ldcw_align (lock); -return val ?0 :EBUSY ; +return __ldcw (a) ?0 :EBUSY ; }weak_alias (__pthread_spin_trylock, pthread_spin_trylock)
@@ -54,7 +47,9 @@int __pthread_spin_unlock (pthread_spinlock_t *lock) { - *lock =1 ; +unsigned int *a =__ldcw_align (lock); + + *a =1 ;return 0 ; }weak_alias (__pthread_spin_unlock, pthread_spin_unlock)
@@ -66,7 +61,9 @@/* We can ignore the `pshared' parameter. Since we are busy-waiting all processes which can access the memory location `lock' points to can use the spinlock. */ - *lock =1 ; +unsigned int *a =__ldcw_align (lock); + + *a =1 ;return 0 ; }weak_alias (__pthread_spin_init, pthread_spin_init)
linuxthreads/sysdeps/hppa/pt-machine.h +67 -14
107 lines not on this page · raw message
linuxthreads/sysdeps/pthread/bits/libc-lock.h +3 -3
@@ -71,12 +71,12 @@initialized locks must be set to one due to the lack of normal atomic operations.) */ -#if __LT_SPINLOCK_INIT ==0 +#ifdef __LT_INITIALIZER_NOT_ZERO# define __libc_lock_define_initialized (CLASS ,NAME ) \ - CLASS __libc_lock_t NAME; + CLASS __libc_lock_t NAME = PTHREAD_MUTEX_INITIALIZER;#else # define __libc_lock_define_initialized (CLASS ,NAME ) \ - CLASS __libc_lock_t NAME = PTHREAD_MUTEX_INITIALIZER; + CLASS __libc_lock_t NAME;#endif #define __libc_rwlock_define_initialized (CLASS ,NAME ) \
linuxthreads/sysdeps/pthread/bits/pthreadtypes.h +5 -3
@@ -22,12 +22,14 @@#define __need_schedparam#include <bits/sched.h> +typedef int __atomic_lock_t ; +/* Fast locks (not abstract because mutexes and conditions aren't abstract). */ struct _pthread_fastlock { -long int __status;/* "Free" or "taken" or head of waiting list */ -int __spinlock;/* Used by compare_and_swap emulation. Also, -adaptive SMP lock stores spin count here. */ +long int __status;/* "Free" or "taken" or head of waiting list */ +__atomic_lock_t __spinlock;/* Used by compare_and_swap emulation. Also, +adaptive SMP lock stores spin count here. */ };#ifndef _PTHREAD_DESCR_DEFINED
linuxthreads/sysdeps/unix/sysv/linux/hppa/bits/initspin.h +17 -5
@@ -17,11 +17,23 @@write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. */ +/* Initialize global spinlocks without cast, generally macro wrapped */ +#define __LT_SPINLOCK_ALT_INIT { { 1, 1, 1, 1, } } +/* Initial value of a spinlock. PA-RISC only implements atomic load and clear so this must be non-zero. */ -#define __LT_SPINLOCK_INIT 1 +#define __LT_SPINLOCK_INIT ((__atomic_lock_t) __LT_SPINLOCK_ALT_INIT) + +/* Macros for lock initializers, not using the above definition. +The above definition is not used in the case that static initializers +use this value. */ +#define __LOCK_INITIALIZER { 0, __LT_SPINLOCK_ALT_INIT } +#define __ATOMIC_INITIALIZER { 0, __LT_SPINLOCK_ALT_INIT } + +/* Used to initialize _pthread_fastlock's in non-static case */ +#define __LOCK_ALT_INITIALIZER ((struct _pthread_fastlock){ 0, __LT_SPINLOCK_INIT }) + +/* Tell the rest of the code that the initializer is non-zero without +explaining it's internal structure */ +#define __LT_INITIALIZER_NOT_ZERO -/* Macros for lock initializers, using the above definition. */ -#define __LOCK_INITIALIZER { 0, __LT_SPINLOCK_INIT } -#define __ALT_LOCK_INITIALIZER { 0, __LT_SPINLOCK_INIT } -#define __ATOMIC_INITIALIZER { 0, __LT_SPINLOCK_INIT }
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 4 lines · Carlos O'Donell
This is a cleaner HPPA linuxthreads implementation that stems from the work that John David Anglin <[email protected]> and myself did to devise a self-aligning lock system that doesn't impose the 16-byte lock alignment restriction.
The indentation and general style is wrong in many places.
linuxthreads/sysdeps/hppa/pspinlock.c line 30
Quoted 12 lines
diff -urN
glibc-2.3.1.orig/linuxthreads/sysdeps/hppa/pspinlock.cglibc-2.3.1/linuxthreads/sysdeps/hppa/pspinlock.clinuxthreads/sysdeps/hppa/pspinlock.c +4
-1@@ -24,15 +24,12 @@int __pthread_spin_lock (pthread_spinlock_t *lock) { -unsigned int val; +unsigned int *addr =__ldcw_align (lock); + +while (__ldcw (addr) ==0 ) +while (*addr ==0 ) ;
This is plain wrong. addr at least must be volatile.
And I don't understand why you removed the asm code. These pieces of code are prime candidates for hand-coding.
linuxthreads/sysdeps/hppa/pspinlock.c
Quoted 6 lines
linuxthreads/sysdeps/hppa/pspinlock.c +6
-0+static inline struct _pthread_descr_struct *__get_cr27 (void ) +{ +long cr27; + asm("mfctl %%cr27, %0" :"=r" (cr27) : ); +return (struct _pthread_descr_struct *) cr27; +}
Not a real problem, but you should get gcc to recognize this reqister and perform the loading.
- --
- --------------. ,-. 444 Castro Street
Ulrich Drepper \ ,-----------------' \ Mountain View, CA 94041 USA
Red Hat --' drepper at redhat.com ---------------------------
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)
iD8DBQE+y74l2ijCOnn/RHQRAvZ1AKCkAhua5qA19EDylfBz2Zhp8dROIACfTrwO 9Slpj32aUy92PiggDNexZBE=
=6Ilf
-----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 4 lines · Carlos O'Donell
This is a cleaner HPPA linuxthreads implementation that stems from the work that John David Anglin <[email protected]> and myself did to devise a self-aligning lock system that doesn't impose the 16-byte lock alignment restriction.
The indentation and general style is wrong in many places.
linuxthreads/sysdeps/hppa/pspinlock.c line 30
Quoted 12 lines
diff -urN
glibc-2.3.1.orig/linuxthreads/sysdeps/hppa/pspinlock.cglibc-2.3.1/linuxthreads/sysdeps/hppa/pspinlock.clinuxthreads/sysdeps/hppa/pspinlock.c +4
-1@@ -24,15 +24,12 @@int __pthread_spin_lock (pthread_spinlock_t *lock) { -unsigned int val; +unsigned int *addr =__ldcw_align (lock); + +while (__ldcw (addr) ==0 ) +while (*addr ==0 ) ;
This is plain wrong. addr at least must be volatile.
And I don't understand why you removed the asm code. These pieces of code are prime candidates for hand-coding.
linuxthreads/sysdeps/hppa/pspinlock.c
Quoted 6 lines
linuxthreads/sysdeps/hppa/pspinlock.c +6
-0+static inline struct _pthread_descr_struct *__get_cr27 (void ) +{ +long cr27; + asm("mfctl %%cr27, %0" :"=r" (cr27) : ); +return (struct _pthread_descr_struct *) cr27; +}
Not a real problem, but you should get gcc to recognize this reqister and perform the loading.
- --
- --------------. ,-. 444 Castro Street
Ulrich Drepper \ ,-----------------' \ Mountain View, CA 94041 USA
Red Hat --' drepper at redhat.com ---------------------------
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)
iD8DBQE+y74l2ijCOnn/RHQRAvZ1AKCkAhua5qA19EDylfBz2Zhp8dROIACfTrwO 9Slpj32aUy92PiggDNexZBE=
=6Ilf
-----END PGP SIGNATURE-----
Ulrich,
Quoted 1 line
The indentation and general style is wrong in many places.
I'll clean this up today and break it into three chunks for even easier digestion.
Quoted 9 lines
linuxthreads/sysdeps/hppa/pspinlock.c line 30
Quoted 7 lines
__pthread_spin_lock(pthread_spinlock_t*lock)
{linuxthreads/sysdeps/hppa/pspinlock.c +4
-1-unsigned int val; +unsigned int *addr =__ldcw_align (lock); + +while (__ldcw (addr) ==0 ) +while (*addr ==0 ) ;This is plain wrong. addr at least must be volatile.
__ldcw is volatile.
Are you suggesting that it is prehaps fragile based on gcc's choice of optimization?
Quoted 2 lines
And I don't understand why you removed the asm code. These pieces of code are prime candidates for hand-coding.
__ldcw is still assembly only. __ldcw_align is a macro. In truth I wanted a functioning implementation first, optimization is a close second. Though a full assembly version would fix gcc from reordering anything.
Quoted 9 lines
linuxthreads/sysdeps/hppa/pt-machine.h line 49
Quoted 6 lines
linuxthreads/sysdeps/hppa/pt-machine.h +6
-0+static inline struct _pthread_descr_struct *__get_cr27 (void ) +{ +long cr27; + asm("mfctl %%cr27, %0" :"=r" (cr27) : ); +return (struct _pthread_descr_struct *) cr27; +}Not a real problem, but you should get gcc to recognize this reqister and perform the loading.
This should get addressed as TLS is implemented for HPPA. I will be attending gcc-summit in hopes to discuss this with my colleagues.
c.
Quoted 6 lines · Carlos O'Donell
Quoted 4 lines
linuxthreads/sysdeps/hppa/pspinlock.c line 30
Quoted 2 lines
linuxthreads/sysdeps/hppa/pspinlock.c +2
-0+while (__ldcw (addr) ==0 ) +while (*addr ==0 ) ;This is plain wrong. addr at least must be volatile.
__ldcwis volatile.
yes, but *addr can be optimized out of the while loop.
Quoted 2 lines
Are you suggesting that it is prehaps fragile based on gcc's choice of optimization?
That's another issue. OTOH, I doubt gcc will ever mangle this fairly short chunk of code. Maybe post the gcc asm output and see if anyone can improve it.
grant
Quoted 9 lines · Carlos O'Donell
Quoted 6 lines
Quoted 4 lines
linuxthreads/sysdeps/hppa/pspinlock.c line 30
Quoted 2 lines
linuxthreads/sysdeps/hppa/pspinlock.c +2
-0+while (__ldcw (addr) ==0 ) +while (*addr ==0 ) ;This is plain wrong. addr at least must be volatile.
__ldcwis volatile.yes, but *addr can be optimized out of the while loop.
Yes, you are correct. This is the code from the inner loop:
1c: 0f 40 10 93 ldw 0(,r26),r19 20: 86 60 3f f5 cmpib,= 0,r19,20 <__pthread_spin_lock+0x20> 24: 08 00 02 40 nop
Don't know how I missied that :(
Dave
Signature · 2 lines
J. David Anglin [email protected] National Research Council of Canada (613) 990-0752 (FAX: 952-6602)
Quoted 9 lines · Carlos O'Donell
Quoted 6 lines
Quoted 4 lines
linuxthreads/sysdeps/hppa/pspinlock.c line 30
Quoted 2 lines
linuxthreads/sysdeps/hppa/pspinlock.c +2
-0+while (__ldcw (addr) ==0 ) +while (*addr ==0 ) ;This is plain wrong. addr at least must be volatile.
__ldcwis volatile.yes, but *addr can be optimized out of the while loop.
Yes, you are correct. This is the code from the inner loop:
1c: 0f 40 10 93 ldw 0(,r26),r19 20: 86 60 3f f5 cmpib,= 0,r19,20 <__pthread_spin_lock+0x20> 24: 08 00 02 40 nop
Don't know how I missied that :(
Dave
Signature · 2 lines
J. David Anglin [email protected] National Research Council of Canada (613) 990-0752 (FAX: 952-6602)
Quoted 1 line · John David Anglin
Don't know how I missied that :(
If you have a test for this, it's not contending for the lock. Writing a test for this would probably be a good thing if there isn't already one. Note there is no fairness and a test would need to be sensitive to that.
grant
Quoted 4 lines
If you have a test for this, it's not contending for the lock. Writing a test for this would probably be a good thing if there isn't already one. Note there is no fairness and a test would need to be sensitive to that.
Agreed. The libc suite would probably be the best place for such a test ;-)
Dave
Signature · 2 lines
J. David Anglin [email protected] National Research Council of Canada (613) 990-0752 (FAX: 952-6602)
Quoted 4 lines
If you have a test for this, it's not contending for the lock. Writing a test for this would probably be a good thing if there isn't already one. Note there is no fairness and a test would need to be sensitive to that.
Agreed. The libc suite would probably be the best place for such a test ;-)
Dave
Signature · 2 lines
J. David Anglin [email protected] National Research Council of Canada (613) 990-0752 (FAX: 952-6602)
Quoted 1 line · John David Anglin
Don't know how I missied that :(
If you have a test for this, it's not contending for the lock. Writing a test for this would probably be a good thing if there isn't already one. Note there is no fairness and a test would need to be sensitive to that.
grant
Quoted 6 lines · Carlos O'Donell
Quoted 4 lines
linuxthreads/sysdeps/hppa/pspinlock.c line 30
Quoted 2 lines
linuxthreads/sysdeps/hppa/pspinlock.c +2
-0+while (__ldcw (addr) ==0 ) +while (*addr ==0 ) ;This is plain wrong. addr at least must be volatile.
__ldcwis volatile.
yes, but *addr can be optimized out of the while loop.
Quoted 2 lines
Are you suggesting that it is prehaps fragile based on gcc's choice of optimization?
That's another issue. OTOH, I doubt gcc will ever mangle this fairly short chunk of code. Maybe post the gcc asm output and see if anyone can improve it.
grant
Ulrich,
Quoted 1 line
The indentation and general style is wrong in many places.
I'll clean this up today and break it into three chunks for even easier digestion.
Quoted 9 lines
linuxthreads/sysdeps/hppa/pspinlock.c line 30
Quoted 7 lines
__pthread_spin_lock(pthread_spinlock_t*lock)
{linuxthreads/sysdeps/hppa/pspinlock.c +4
-1-unsigned int val; +unsigned int *addr =__ldcw_align (lock); + +while (__ldcw (addr) ==0 ) +while (*addr ==0 ) ;This is plain wrong. addr at least must be volatile.
__ldcw is volatile.
Are you suggesting that it is prehaps fragile based on gcc's choice of optimization?
Quoted 2 lines
And I don't understand why you removed the asm code. These pieces of code are prime candidates for hand-coding.
__ldcw is still assembly only. __ldcw_align is a macro. In truth I wanted a functioning implementation first, optimization is a close second. Though a full assembly version would fix gcc from reordering anything.
Quoted 9 lines
linuxthreads/sysdeps/hppa/pt-machine.h line 49
Quoted 6 lines
linuxthreads/sysdeps/hppa/pt-machine.h +6
-0+static inline struct _pthread_descr_struct *__get_cr27 (void ) +{ +long cr27; + asm("mfctl %%cr27, %0" :"=r" (cr27) : ); +return (struct _pthread_descr_struct *) cr27; +}Not a real problem, but you should get gcc to recognize this reqister and perform the loading.
This should get addressed as TLS is implemented for HPPA. I will be attending gcc-summit in hopes to discuss this with my colleagues.
c.