|
php.net | support | documentation | report a bug | advanced search | search howto | statistics | random bug | login |
[2010-11-15 00:21 UTC] stefan at whocares dot de
Description:
------------
Compiling with PHP-FPM enabled on an older SPARC system will result in
/tmp/cc6w5Fh0.s: Assembler messages:
/tmp/cc6w5Fh0.s:39: Error: Architecture mismatch on "cas".
/tmp/cc6w5Fh0.s:39: (Requires v9|v9a|v9b; requested architecture is sparclite.)
Unfortunately my knowledge of SPARC assembly language isn't nearly good enough to fix that. I know that the v9 "cas" opcode does an atomic "compare and swap" operation but I wouldn't know how to translate that into v8 code.
Test script:
---------------
Copy /sapi/fpm/fpm/fpm_atomic.h to fpm_atomic.c and add bogus main() function:
int main () {
int result;
atomic_t mylock;
result = fpm_spinlock(&mylock, 1);
}
Compile using "gcc -mcpu=v8 fpm_atomic.c" will result in error message given.
Expected result:
----------------
Should compile without error.
Actual result:
--------------
sparky:~# gcc -mcpu=v8 fpm_atomic.c
/tmp/cciAbMrC.s: Assembler messages:
/tmp/cciAbMrC.s:121: Error: Architecture mismatch on "cas".
/tmp/cciAbMrC.s:121: (Requires v9|v9a|v9b; requested architecture is sparclite.)
sparky:~#
Patchesfpm_atomics_sparc_v8.patch (last revision 2010-11-15 02:04 UTC by stefan at whocares dot de)Pull RequestsHistoryAllCommentsChangesGit/SVN commits
|
|||||||||||||||||||||||||||||||||||||
Copyright © 2001-2025 The PHP GroupAll rights reserved. |
Last updated: Tue Oct 28 06:00:01 2025 UTC |
Well, I blatantly copied from PostgreSQL's s_lock.h and came up with this: diff -Nau fpm_atomic.h.org fpm_atomic.h --- fpm_atomic.h.org 2009-12-14 09:18:53.000000000 +0000 +++ fpm_atomic.h 2010-11-15 01:50:31.000000000 +0000 @@ -82,7 +82,7 @@ #endif /* defined (__GNUC__) &&... */ #elif ( __sparc__ || __sparc ) /* Marcin Ochab */ - +#if (__sparc_v9__) #if (__arch64__ || __arch64) typedef uint64_t atomic_uint_t; typedef volatile atomic_uint_t atomic_t; @@ -118,7 +118,23 @@ } /* }}} */ #endif +#else /* sparcv9 */ +typedef uint32_t atomic_uint_t; +typedef volatile atomic_uint_t atomic_t; +static inline int atomic_cas_32(atomic_t *lock) /* {{{ */ +{ + register atomic_uint_t _res; + __asm__ __volatile__("ldstub [%2], %0" : "=r"(_res), "+m"(*lock) : "r"(lock) : "memory"); + return (int) _res; +} +/* }}} */ + +static inline atomic_uint_t atomic_cmp_set(atomic_t *lock, atomic_uint_t old, atomic_uint_t set) /* {{{ */ +{ + return (atomic_cas_32(lock)==0); +} +/* }}} */ #else #error unsupported processor. please write a patch and send it to me Rationale: If I'm reading the original code correctly, there's no actual locking done but instead the code only tests whether it could acquire a lock. 'ldstub' works such that it returns the current value of the memory region specified and sets it to all '1' afterwards. Thus, if the return value is '-1' the lock was already set by another process whereas if it's '0' we acquired the lock. Well, at least in my certainly flawed logic ;) Since ldstub is atomic I didn't see a need to explicitly "lock;" the code. The patch should leave the 'cas' code intact when being compiled on v9 type SPARC systems. Tested (for successful compilation only!) on Debian (etch) using gcc 3.3.5. Thus I believe further testing is necessary to verify this is actually working. Well, please test and incorporate if you feel the code is doing what it's supposed to do.As you may have read in my initial post, the compiler I (have to) use is gcc 3.3.5 which falls a bit short of 4.1 ;) Also, you may want to read the backend/port/tas/solaris_sparc.s file from the official PostgreSQL sources: ! "cas" only works on sparcv9 and sparcv8plus chips, and ! requies a compiler targeting these CPUs. It will fail ! on a compiler targeting sparcv8, and of course will not ! be understood by a sparcv8 CPU. gcc continues to use ! "ldstub" because it targets sparcv7. There they work around this by using a condition (for the SUN compiler) like this: #if defined(__sparcv9) || defined(__sparcv8plus) cas [%o0],%o2,%o1 #else ldstub [%o0],%o1 #endif and in their actual generic lock implementation (src/include/storage/s_lock.h) the code is this: #if defined(__sparc__) /* Sparc */ #define HAS_TEST_AND_SET typedef unsigned char slock_t; #define TAS(lock) tas(lock) static __inline__ int tas(volatile slock_t *lock) { register slock_t _res; /* * See comment in /pg/backend/port/tas/solaris_sparc.s for why this * uses "ldstub", and that file uses "cas". gcc currently generates * sparcv7-targeted binaries, so "cas" use isn't possible. */ __asm__ __volatile__( " ldstub [%2], %0 \n" : "=r"(_res), "+m"(*lock) : "r"(lock) : "memory"); return (int) _res; } #endif /* __sparc__ */ Now my general idea was that if there's a reason for PostgreSQL to keep that code around, there might be a reason for PHP to do so as well. Obviously I was wrong there. I also do not see the real advantage of 'cas' over 'ldstub' in the current scenario since both are atomic, both are supported (ldstub even on v7) and both do the job perfectly well.Thanks for your input, greatly appreciated. It'd be nice if you could help me out a bit more. I grepped through the whole source and the only use of 'atomic_cmp_set' I could find was within fpm_atomic.h itself: (333)sparky:~/devel/build/php5-5.3.3# grep -R 'atomic_cmp_set' * sapi/fpm/fpm/fpm_atomic.h:static inline atomic_uint_t atomic_cmp_set(atomic_t *lock, atomic_uint_t old, atomic_uint_t set) /* {{{ */ sapi/fpm/fpm/fpm_atomic.h:static inline atomic_uint_t atomic_cmp_set(atomic_t *lock, atomic_uint_t old, atomic_uint_t set) /* {{{ */ sapi/fpm/fpm/fpm_atomic.h:static inline atomic_uint_t atomic_cmp_set(atomic_t *lock, atomic_uint_t old, atomic_uint_t set) /* {{{ */ sapi/fpm/fpm/fpm_atomic.h:static inline atomic_uint_t atomic_cmp_set(atomic_t *lock, atomic_uint_t old, atomic_uint_t set) /* {{{ */ sapi/fpm/fpm/fpm_atomic.h:static inline atomic_uint_t atomic_cmp_set(atomic_t *lock, atomic_uint_t old, atomic_uint_t set) /* {{{ */ sapi/fpm/fpm/fpm_atomic.h: return atomic_cmp_set(lock, 0, 1) ? 0 : -1; sapi/fpm/fpm/fpm_atomic.h: if (atomic_cmp_set(lock, 0, 1)) { (333)sparky:~/devel/build/php5-5.3.3# Since you're saying it's a general purpose function I'm obviously missing something here and because I'm an enquiring mind I'd like to know what it is I'm missing. I agree that using C code would improve portability. But since the machine I'm building for is slow enough already, I'd prefer to stick with assembler if possible,