Thread: s_lock.h cleanup
In looking at the VAX ASM problem, I realized that the ASM in s_lock.h
is all formatted differently, making it even more confusing. I have
applied the following patch to s_lock.h to try and clean it up.
The new standard format is:
/*
* Standard __asm__ format:
*
* __asm__(
* "command;"
* "command;"
* "command;"
* : "=r"(_res) return value, in register
* : "r"(lock) argument, 'lock pointer', in register
* : "r0"); inline code uses this register
*/
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
? config.log
? config.cache
? config.status
? GNUmakefile
? src/Makefile.custom
? src/GNUmakefile
? src/Makefile.global
? src/log
? src/crtags
? src/backend/postgres
? src/backend/catalog/global.bki
? src/backend/catalog/global.description
? src/backend/catalog/template1.bki
? src/backend/catalog/template1.description
? src/backend/port/Makefile
? src/bin/initdb/initdb
? src/bin/initlocation/initlocation
? src/bin/ipcclean/ipcclean
? src/bin/pg_config/pg_config
? src/bin/pg_ctl/pg_ctl
? src/bin/pg_dump/pg_dump
? src/bin/pg_dump/pg_restore
? src/bin/pg_dump/pg_dumpall
? src/bin/pg_id/pg_id
? src/bin/pg_passwd/pg_passwd
? src/bin/pgaccess/pgaccess
? src/bin/pgtclsh/Makefile.tkdefs
? src/bin/pgtclsh/Makefile.tcldefs
? src/bin/pgtclsh/pgtclsh
? src/bin/pgtclsh/pgtksh
? src/bin/psql/psql
? src/bin/scripts/createlang
? src/include/config.h
? src/include/stamp-h
? src/interfaces/ecpg/lib/libecpg.so.3.2.0
? src/interfaces/ecpg/preproc/ecpg
? src/interfaces/libpgeasy/libpgeasy.so.2.1
? src/interfaces/libpgtcl/libpgtcl.so.2.1
? src/interfaces/libpq/libpq.so.2.1
? src/interfaces/perl5/blib
? src/interfaces/perl5/Makefile
? src/interfaces/perl5/pm_to_blib
? src/interfaces/perl5/Pg.c
? src/interfaces/perl5/Pg.bs
? src/pl/plperl/blib
? src/pl/plperl/Makefile
? src/pl/plperl/pm_to_blib
? src/pl/plperl/SPI.c
? src/pl/plperl/plperl.bs
? src/pl/plpgsql/src/libplpgsql.so.1.0
? src/pl/tcl/Makefile.tcldefs
Index: src/include/storage/s_lock.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/storage/s_lock.h,v
retrieving revision 1.78
diff -c -r1.78 s_lock.h
*** src/include/storage/s_lock.h 2001/01/18 23:40:26 1.78
--- src/include/storage/s_lock.h 2001/01/19 02:52:49
***************
*** 35,41 ****
*
* int TAS(slock_t *lock)
* Atomic test-and-set instruction. Attempt to acquire the lock,
! * but do *not* wait. Returns 0 if successful, nonzero if unable
* to acquire the lock.
*
* TAS() is a lower-level part of the API, but is used directly in a
--- 35,41 ----
*
* int TAS(slock_t *lock)
* Atomic test-and-set instruction. Attempt to acquire the lock,
! * but do *not* wait. Returns 0 if successful, nonzero if unable
* to acquire the lock.
*
* TAS() is a lower-level part of the API, but is used directly in a
***************
*** 48,56 ****
* unsigned spins = 0;
*
* while (TAS(lock))
- * {
* S_LOCK_SLEEP(lock, spins++);
- * }
* }
*
* where S_LOCK_SLEEP() checks for timeout and sleeps for a short
--- 48,54 ----
***************
*** 87,96 ****
/* Platform-independent out-of-line support routines */
extern void s_lock(volatile slock_t *lock,
! const char *file, const int line);
extern void s_lock_sleep(unsigned spins, int microsec,
! volatile slock_t *lock,
! const char *file, const int line);
#if defined(HAS_TEST_AND_SET)
--- 85,94 ----
/* Platform-independent out-of-line support routines */
extern void s_lock(volatile slock_t *lock,
! const char *file, const int line);
extern void s_lock_sleep(unsigned spins, int microsec,
! volatile slock_t *lock,
! const char *file, const int line);
#if defined(HAS_TEST_AND_SET)
***************
*** 101,106 ****
--- 99,116 ----
* All the gcc inlines
*/
+ /*
+ * Standard __asm__ format:
+ *
+ * __asm__(
+ * "command;"
+ * "command;"
+ * "command;"
+ * : "=r"(_res) return value, in register
+ * : "r"(lock) argument, 'lock pointer', in register
+ * : "r0"); inline code uses this register
+ */
+
#if defined(__i386__)
#define TAS(lock) tas(lock)
***************
*** 110,116 ****
{
register slock_t _res = 1;
! __asm__("lock; xchgb %0,%1": "=q"(_res), "=m"(*lock):"0"(_res));
return (int) _res;
}
--- 120,130 ----
{
register slock_t _res = 1;
! __asm__(
! "lock;"
! "xchgb %0,%1;"
! : "=q"(_res), "=m"(*lock)
! : "0"(_res));
return (int) _res;
}
***************
*** 121,139 ****
#define TAS(lock) tas(lock)
static __inline__ int
! tas (volatile slock_t *lock)
{
! long int ret;
! __asm__ __volatile__(
! "xchg4 %0=%1,%2"
! : "=r"(ret), "=m"(*lock)
! : "r"(1), "1"(*lock)
! : "memory");
! return (int) ret;
}
! #endif /* __ia64__ */
#if defined(__arm__) || defined(__arm__)
--- 135,154 ----
#define TAS(lock) tas(lock)
static __inline__ int
! tas(volatile slock_t *lock)
{
! long int ret;
! __asm__ __volatile__(
! "xchg4 %0=%1,%2;"
! : "=r"(ret), "=m"(*lock)
! : "r"(1), "1"(*lock)
! : "memory");
! return (int) ret;
}
!
! #endif /* __ia64__ */
#if defined(__arm__) || defined(__arm__)
***************
*** 142,180 ****
static __inline__ int
tas(volatile slock_t *lock)
{
! register slock_t _res = 1;
! __asm__("swpb %0, %0, [%3]": "=r"(_res), "=m"(*lock):"0"(_res), "r" (lock));
! return (int) _res;
}
! #endif /* __arm__ */
#if defined(__s390__)
/*
* S/390 Linux
*/
! #define TAS(lock) tas(lock)
static inline int
tas(volatile slock_t *lock)
{
! int _res;
! __asm__ __volatile(" la 1,1\n"
! " l 2,%2\n"
! " slr 0,0\n"
! " cs 0,1,0(2)\n"
! " lr %1,0"
! : "=m" (lock), "=d" (_res)
! : "m" (lock)
! : "0", "1", "2");
! return (_res);
}
- #endif /* __s390__ */
#if defined(__sparc__)
#define TAS(lock) tas(lock)
--- 157,200 ----
static __inline__ int
tas(volatile slock_t *lock)
{
! register slock_t _res = 1;
! __asm__(
! "swpb %0, %0, [%3];"
! : "=r"(_res), "=m"(*lock)
! : "0"(_res), "r"(lock));
! return (int) _res;
}
! #endif /* __arm__ */
#if defined(__s390__)
/*
* S/390 Linux
*/
! #define TAS(lock) tas(lock)
static inline int
tas(volatile slock_t *lock)
{
! int _res;
! __asm__ __volatile(
! "la 1,1;"
! "l 2,%2;"
! "slr 0,0;"
! "cs 0,1,0(2);"
! "lr %1,0;"
! : "=m"(lock), "=d"(_res)
! : "m"(lock)
! : "0", "1", "2");
! return (_res);
}
+ #endif /* __s390__ */
+
#if defined(__sparc__)
#define TAS(lock) tas(lock)
***************
*** 183,190 ****
{
register slock_t _res = 1;
! __asm__("ldstub [%2], %0" \
! : "=r"(_res), "=m"(*lock) \
: "r"(lock));
return (int) _res;
}
--- 203,211 ----
{
register slock_t _res = 1;
! __asm__(
! "ldstub [%2], %0;"
! : "=r"(_res), "=m"(*lock)
: "r"(lock));
return (int) _res;
}
***************
*** 199,214 ****
tas(volatile slock_t *lock)
{
register int rv;
!
! __asm__ __volatile__ (
! "tas %1; sne %0"
! : "=d" (rv), "=m"(*lock)
! : "1" (*lock)
! : "cc" );
return rv;
}
! #endif /* defined(__mc68000__) && defined(__linux__) */
#if defined(NEED_VAX_TAS_ASM)
--- 220,237 ----
tas(volatile slock_t *lock)
{
register int rv;
!
! __asm__ __volatile__(
! "tas %1;"
! "sne %0;"
! : "=d"(rv), "=m"(*lock)
! : "1"(*lock)
! : "cc");
!
return rv;
}
! #endif /* defined(__mc68000__) && defined(__linux__) */
#if defined(NEED_VAX_TAS_ASM)
***************
*** 225,237 ****
{
register _res;
! __asm__(" movl $1, r0 \
! bbssi $0, (%1), 1f \
! clrl r0 \
! 1: movl r0, %0 "
! : "=r"(_res) /* return value, in register */
! : "r"(lock) /* argument, 'lock pointer', in register */
! : "r0"); /* inline code uses this register */
return (int) _res;
}
--- 248,261 ----
{
register _res;
! __asm__(
! "movl $1, r0;"
! "bbssi $0, (%1), 1f;"
! "clrl r0;"
! "1: movl r0, %0;"
! : "=r"(_res)
! : "r"(lock)
! : "r0");
return (int) _res;
}
***************
*** 244,257 ****
static __inline__ int
tas(volatile slock_t *lock)
{
! register _res;
! __asm__("sbitb 0, %0 \n\
! sfsd %1"
! : "=m"(*lock), "=r"(_res));
! return (int) _res;
}
! #endif /* NEED_NS32K_TAS_ASM */
--- 268,283 ----
static __inline__ int
tas(volatile slock_t *lock)
{
! register _res;
!
! __asm__(
! "sbitb 0, %0;"
! "sfsd %1;"
! : "=m"(*lock), "=r"(_res));
! return (int) _res;
}
! #endif /* NEED_NS32K_TAS_ASM */
***************
*** 268,274 ****
tas(volatile slock_t *s_lock)
{
/* UNIVEL wants %mem in column 1, so we don't pg_indent this file */
! %mem s_lock
pushl %ebx
movl s_lock, %ebx
movl $255, %eax
--- 294,300 ----
tas(volatile slock_t *s_lock)
{
/* UNIVEL wants %mem in column 1, so we don't pg_indent this file */
! %mem s_lock
pushl %ebx
movl s_lock, %ebx
movl $255, %eax
***************
*** 277,283 ****
popl %ebx
}
! #endif /* defined(NEED_I386_TAS_ASM) && defined(USE_UNIVEL_CC) */
#endif /* defined(__GNUC__) */
--- 303,309 ----
popl %ebx
}
! #endif /* defined(NEED_I386_TAS_ASM) && defined(USE_UNIVEL_CC) */
#endif /* defined(__GNUC__) */
***************
*** 300,329 ****
#if defined(__GNUC__)
#define TAS(lock) tas(lock)
! #define S_UNLOCK(lock) do { __asm__ volatile ("mb"); *(lock) = 0; } while (0)
static __inline__ int
tas(volatile slock_t *lock)
{
register slock_t _res;
! __asm__ volatile
! (" ldq $0, %0 \n\
! bne $0, 2f \n\
! ldq_l %1, %0 \n\
! bne %1, 2f \n\
! mov 1, $0 \n\
! stq_c $0, %0 \n\
! beq $0, 2f \n\
! mb \n\
! br 3f \n\
! 2: mov 1, %1 \n\
! 3: \n" : "=m"(*lock), "=r"(_res) : : "0");
return (int) _res;
}
! #else /* !defined(__GNUC__) */
/*
* The Tru64 compiler doesn't support gcc-style inline asm, but it does
--- 326,358 ----
#if defined(__GNUC__)
#define TAS(lock) tas(lock)
! #define S_UNLOCK(lock) do { __asm__ volatile ("mb"); *(lock) = 0; } while (0)
static __inline__ int
tas(volatile slock_t *lock)
{
register slock_t _res;
! __asm__ volatile(
! "ldq $0, %0;"
! "bne $0, 2f;"
! "ldq_l %1, %0;"
! "bne %1, 2f;"
! "mov 1, $0;"
! "stq_c $0, %0;"
! "beq $0, 2f;"
! "mb;"
! "br 3f;"
! "2: mov 1, %1;"
! "3:"
! : "=m"(*lock), "=r"(_res)
! :
! : "0");
return (int) _res;
}
! #else /* !defined(__GNUC__) */
/*
* The Tru64 compiler doesn't support gcc-style inline asm, but it does
***************
*** 337,348 ****
#include <alpha/builtins.h>
#define S_INIT_LOCK(lock) (*(lock) = 0)
! #define TAS(lock) (__LOCK_LONG_RETRY((lock), 1) == 0)
! #define S_UNLOCK(lock) __UNLOCK_LONG(lock)
! #endif /* defined(__GNUC__) */
! #endif /* __alpha */
#if defined(__hpux)
--- 366,377 ----
#include <alpha/builtins.h>
#define S_INIT_LOCK(lock) (*(lock) = 0)
! #define TAS(lock) (__LOCK_LONG_RETRY((lock), 1) == 0)
! #define S_UNLOCK(lock) __UNLOCK_LONG(lock)
! #endif /* defined(__GNUC__) */
! #endif /* __alpha */
#if defined(__hpux)
***************
*** 373,390 ****
*
* Note that slock_t under QNX is sem_t instead of char
*/
! #define TAS(lock) (sem_trywait((lock)) < 0)
! #define S_UNLOCK(lock) sem_post((lock))
! #define S_INIT_LOCK(lock) sem_init((lock), 1, 1)
! #define S_LOCK_FREE(lock) ((lock)->value)
! #endif /* __QNX__ */
#if defined(__sgi)
/*
* SGI IRIX 5
* slock_t is defined as a unsigned long. We use the standard SGI
! * mutex API.
*
* The following comment is left for historical reasons, but is probably
* not a good idea since the mutex ABI is supported.
--- 402,419 ----
*
* Note that slock_t under QNX is sem_t instead of char
*/
! #define TAS(lock) (sem_trywait((lock)) < 0)
! #define S_UNLOCK(lock) sem_post((lock))
! #define S_INIT_LOCK(lock) sem_init((lock), 1, 1)
! #define S_LOCK_FREE(lock) ((lock)->value)
! #endif /* __QNX__ */
#if defined(__sgi)
/*
* SGI IRIX 5
* slock_t is defined as a unsigned long. We use the standard SGI
! * mutex API.
*
* The following comment is left for historical reasons, but is probably
* not a good idea since the mutex ABI is supported.
***************
*** 402,408 ****
#if defined(sinix)
/*
! * SINIX / Reliant UNIX
* slock_t is defined as a struct abilock_t, which has a single unsigned long
* member. (Basically same as SGI)
*
--- 431,437 ----
#if defined(sinix)
/*
! * SINIX / Reliant UNIX
* slock_t is defined as a struct abilock_t, which has a single unsigned long
* member. (Basically same as SGI)
*
***************
*** 412,418 ****
#define S_INIT_LOCK(lock) init_lock(lock)
#define S_LOCK_FREE(lock) (stat_lock(lock) == UNLOCKED)
#endif /* sinix */
!
#if defined(_AIX)
/*
--- 441,447 ----
#define S_INIT_LOCK(lock) init_lock(lock)
#define S_LOCK_FREE(lock) (stat_lock(lock) == UNLOCKED)
#endif /* sinix */
!
#if defined(_AIX)
/*
***************
*** 440,446 ****
! #else /* !HAS_TEST_AND_SET */
/*
* Fake spinlock implementation using SysV semaphores --- slow and prone
--- 469,475 ----
! #else /* !HAS_TEST_AND_SET */
/*
* Fake spinlock implementation using SysV semaphores --- slow and prone
***************
*** 451,469 ****
typedef struct
{
/* reference to semaphore used to implement this spinlock */
! IpcSemaphoreId semId;
! int sem;
} slock_t;
extern bool s_lock_free_sema(volatile slock_t *lock);
extern void s_unlock_sema(volatile slock_t *lock);
extern void s_init_lock_sema(volatile slock_t *lock);
! extern int tas_sema(volatile slock_t *lock);
! #define S_LOCK_FREE(lock) s_lock_free_sema(lock)
! #define S_UNLOCK(lock) s_unlock_sema(lock)
! #define S_INIT_LOCK(lock) s_init_lock_sema(lock)
! #define TAS(lock) tas_sema(lock)
#endif /* HAS_TEST_AND_SET */
--- 480,498 ----
typedef struct
{
/* reference to semaphore used to implement this spinlock */
! IpcSemaphoreId semId;
! int sem;
} slock_t;
extern bool s_lock_free_sema(volatile slock_t *lock);
extern void s_unlock_sema(volatile slock_t *lock);
extern void s_init_lock_sema(volatile slock_t *lock);
! extern int tas_sema(volatile slock_t *lock);
! #define S_LOCK_FREE(lock) s_lock_free_sema(lock)
! #define S_UNLOCK(lock) s_unlock_sema(lock)
! #define S_INIT_LOCK(lock) s_init_lock_sema(lock)
! #define TAS(lock) tas_sema(lock)
#endif /* HAS_TEST_AND_SET */
As long as we're cleaning things up, I would suggest that all the ports
that use gcc assembler be made to declare it uniformly, as
__asm__ __volatile__ ( ... );
As I read the GCC manual, there's some risk of the asm sections getting
moved around in the program flow if they are not marked volatile. Also
we oughta be consistent about using the double-underscore keywords IMHO.
regards, tom lane
Done and applied. > As long as we're cleaning things up, I would suggest that all the ports > that use gcc assembler be made to declare it uniformly, as > > __asm__ __volatile__ ( ... ); > > As I read the GCC manual, there's some risk of the asm sections getting > moved around in the program flow if they are not marked volatile. Also > we oughta be consistent about using the double-underscore keywords IMHO. > > regards, tom lane > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Bruce Momjian writes: > In looking at the VAX ASM problem, I realized that the ASM in s_lock.h > is all formatted differently, making it even more confusing. I have > applied the following patch to s_lock.h to try and clean it up. I don't believe in this patch at all. It makes the assumption that all assemblers have equally forgiving lexical rules as a certain subset of said assemblers. For example, the VAX code does not look at all like the one back when it still worked. -- Peter Eisentraut peter_e@gmx.net http://yi.org/peter-e/
> Bruce Momjian writes: > > > In looking at the VAX ASM problem, I realized that the ASM in s_lock.h > > is all formatted differently, making it even more confusing. I have > > applied the following patch to s_lock.h to try and clean it up. > > I don't believe in this patch at all. It makes the assumption that all > assemblers have equally forgiving lexical rules as a certain subset of > said assemblers. For example, the VAX code does not look at all like the > one back when it still worked. I agree the VAX code was changed in the patch, but the VAX person sent email that he had to add the semicolons to make it work on his platform, and that the original " \n\" code did compile at all. I believe the formatting problem was that some code had "command;command; : lkjasfd : asldfk" while some had them spread over separate lines, and others used \n\, all very randomly. Now at least they are all consistent and use similar formatting. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Peter Eisentraut <peter_e@gmx.net> writes:
> Bruce Momjian writes:
>> In looking at the VAX ASM problem, I realized that the ASM in s_lock.h
>> is all formatted differently, making it even more confusing. I have
>> applied the following patch to s_lock.h to try and clean it up.
> I don't believe in this patch at all. It makes the assumption that all
> assemblers have equally forgiving lexical rules as a certain subset of
> said assemblers. For example, the VAX code does not look at all like the
> one back when it still worked.
Good point. I think it's safe to use the split-up-string-literal
feature, but assuming that ';' can replace '\n' is sheer folly, and so
is assuming that whitespace doesn't matter (ie, that opcodes starting
in column 1 are OK). Bruce, I'd suggest a format more like
"[label] opcode operands \n"
for each line of assembly code.
regards, tom lane
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I believe the formatting problem was that some code had
> "command;command; : lkjasfd : asldfk" while some had them spread over
> separate lines, and others used \n\, all very randomly. Now at least
> they are all consistent and use similar formatting.
And they may all be broken, except for the one(s) you have tested.
You shouldn't be assuming that a platform that uses gcc necessarily
also uses gas.
regards, tom lane
> Peter Eisentraut <peter_e@gmx.net> writes: > > Bruce Momjian writes: > >> In looking at the VAX ASM problem, I realized that the ASM in s_lock.h > >> is all formatted differently, making it even more confusing. I have > >> applied the following patch to s_lock.h to try and clean it up. > > > I don't believe in this patch at all. It makes the assumption that all > > assemblers have equally forgiving lexical rules as a certain subset of > > said assemblers. For example, the VAX code does not look at all like the > > one back when it still worked. > > Good point. I think it's safe to use the split-up-string-literal > feature, but assuming that ';' can replace '\n' is sheer folly, and so > is assuming that whitespace doesn't matter (ie, that opcodes starting > in column 1 are OK). Bruce, I'd suggest a format more like > > "[label] opcode operands \n" > > for each line of assembly code. Interestingly, we have very few non-gcc ASM entries in s_lock.h. The only non-gcc one I see are Univel/i386, and I didn't touch that. Isn't the semicolon the standard command terminator for all gcc assemblers? I see non-gcc stuff in s_lock.c, but I didn't touch that. I also see volatile missing in s_lock.c, which I will add for GCC entries. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
> Bruce Momjian <pgman@candle.pha.pa.us> writes: > > I believe the formatting problem was that some code had > > "command;command; : lkjasfd : asldfk" while some had them spread over > > separate lines, and others used \n\, all very randomly. Now at least > > they are all consistent and use similar formatting. > > And they may all be broken, except for the one(s) you have tested. > You shouldn't be assuming that a platform that uses gcc necessarily > also uses gas. Oh, wow, I never suspected gcc could work without gas. Can it? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
> Bruce Momjian <pgman@candle.pha.pa.us> writes: > > I believe the formatting problem was that some code had > > "command;command; : lkjasfd : asldfk" while some had them spread over > > separate lines, and others used \n\, all very randomly. Now at least > > they are all consistent and use similar formatting. > > And they may all be broken, except for the one(s) you have tested. > You shouldn't be assuming that a platform that uses gcc necessarily > also uses gas. > > regards, tom lane I can tell you that they all used __asm__, and all used the colon terminators for each __asm__ block: * __asm__ __volatile__(* "command;"* "command;"* "command;"* : "=r"(_res) returnvalue, in register* : "r"(lock) argument, 'lock pointer', in register* : "r0"); inline code uses this register -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> And they may all be broken, except for the one(s) you have tested.
>> You shouldn't be assuming that a platform that uses gcc necessarily
>> also uses gas.
> I can tell you that they all used __asm__, and all used the colon
> terminators for each __asm__ block:
> * __asm__ __volatile__(
> * "command;"
> * "command;"
> * "command;"
> * : "=r"(_res) return value, in register
> * : "r"(lock) argument, 'lock pointer', in register
> * : "r0"); inline code uses this register
The __asm___ and splitting up the assembly code into multiple string
literals and the consistent formatting of the register addendums are
all fine, because those are read by gcc and this whole code block is
gcc-only. But the assembly code string literal will be spit out
essentially verbatim by gcc to the assembler, and the assembler may
not be nearly as forgiving as you think.
> Oh, wow, I never suspected gcc could work without gas. Can it?
Gcc with platform-specific as used to be the standard configuration
on HPUX, and may still be standard on some platforms.
Bottom line: I see no point in taking any risks, especially not this
late in beta, with code that you cannot test for yourself, and
*especially* not when the change is only for cosmetic reasons.
regards, tom lane
* Tom Lane <tgl@sss.pgh.pa.us> [010119 13:08]: > > > Oh, wow, I never suspected gcc could work without gas. Can it? > > Gcc with platform-specific as used to be the standard configuration > on HPUX, and may still be standard on some platforms. Still is the standard on UnixWare with GCC. The standard assembler and linker are used. > > Bottom line: I see no point in taking any risks, especially not this > late in beta, with code that you cannot test for yourself, and > *especially* not when the change is only for cosmetic reasons. I agree with this sentiment. LER -- Larry Rosenman http://www.lerctr.org/~ler Phone: +1 972-414-9812 E-Mail: ler@lerctr.org US Mail: 1905 Steamboat Springs Drive, Garland, TX 75044-6749
Bruce Momjian <pgman@candle.pha.pa.us> writes: > Interestingly, we have very few non-gcc ASM entries in s_lock.h. The > only non-gcc one I see are Univel/i386, and I didn't touch that. Isn't > the semicolon the standard command terminator for all gcc assemblers? No. It is for most, but not for the a29k, AVR, CRIS, d10v, d30v, FR30, H8/300, HP/PA, TIC30, TIC54x, or TIC80. Aren't you glad you know that now? Ian Former GNU binutils maintainer
Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > > I believe the formatting problem was that some code had > > > "command;command; : lkjasfd : asldfk" while some had them spread over > > > separate lines, and others used \n\, all very randomly. Now at least > > > they are all consistent and use similar formatting. > > > > And they may all be broken, except for the one(s) you have tested. > > You shouldn't be assuming that a platform that uses gcc necessarily > > also uses gas. > > Oh, wow, I never suspected gcc could work without gas. Can it? Yes. In fact, I don't think there is any Unix system on which gcc requires gas. There used to be at least one, but I think they have all been cleaned up at this point. Ian
> The __asm___ and splitting up the assembly code into multiple string
> literals and the consistent formatting of the register addendums are
> all fine, because those are read by gcc and this whole code block is
> gcc-only. But the assembly code string literal will be spit out
> essentially verbatim by gcc to the assembler, and the assembler may
> not be nearly as forgiving as you think.
>
> > Oh, wow, I never suspected gcc could work without gas. Can it?
>
> Gcc with platform-specific as used to be the standard configuration
> on HPUX, and may still be standard on some platforms.
>
> Bottom line: I see no point in taking any risks, especially not this
> late in beta, with code that you cannot test for yourself, and
> *especially* not when the change is only for cosmetic reasons.
OK, remove semicolons and put back the \n at the end of each line.
Patch attached.
I wasn't going to mess with this while in beta, but when I found the VAX
code broken, it seemed worth making sure they were all OK. The VAX
stuff was broken because in 7.0.3 it shows:
__asm__(" movl $1, r0 \
bbssi $0, (%1), 1 f \
clrl r0 \
1: movl r0, %0 "
The '1 f' we broken, but also the thing missing here is \n\. With \, it
just makes one long line, which certainly can't be asembled. The VAX
guy added semicolons, but I can see that \n\ is safer, and have done
that.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
? config.log
? config.cache
? config.status
? GNUmakefile
? src/Makefile.custom
? src/GNUmakefile
? src/Makefile.global
? src/log
? src/crtags
? src/backend/postgres
? src/backend/catalog/global.bki
? src/backend/catalog/global.description
? src/backend/catalog/template1.bki
? src/backend/catalog/template1.description
? src/backend/port/Makefile
? src/bin/initdb/initdb
? src/bin/initlocation/initlocation
? src/bin/ipcclean/ipcclean
? src/bin/pg_config/pg_config
? src/bin/pg_ctl/pg_ctl
? src/bin/pg_dump/pg_dump
? src/bin/pg_dump/pg_restore
? src/bin/pg_dump/pg_dumpall
? src/bin/pg_id/pg_id
? src/bin/pg_passwd/pg_passwd
? src/bin/pgaccess/pgaccess
? src/bin/pgtclsh/Makefile.tkdefs
? src/bin/pgtclsh/Makefile.tcldefs
? src/bin/pgtclsh/pgtclsh
? src/bin/pgtclsh/pgtksh
? src/bin/psql/psql
? src/bin/scripts/createlang
? src/include/config.h
? src/include/stamp-h
? src/interfaces/ecpg/lib/libecpg.so.3.2.0
? src/interfaces/ecpg/preproc/ecpg
? src/interfaces/libpgeasy/libpgeasy.so.2.1
? src/interfaces/libpgtcl/libpgtcl.so.2.1
? src/interfaces/libpq/libpq.so.2.1
? src/interfaces/perl5/blib
? src/interfaces/perl5/Makefile
? src/interfaces/perl5/pm_to_blib
? src/interfaces/perl5/Pg.c
? src/interfaces/perl5/Pg.bs
? src/pl/plperl/blib
? src/pl/plperl/Makefile
? src/pl/plperl/pm_to_blib
? src/pl/plperl/SPI.c
? src/pl/plperl/plperl.bs
? src/pl/plpgsql/src/libplpgsql.so.1.0
? src/pl/tcl/Makefile.tcldefs
Index: src/backend/storage/buffer/s_lock.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/storage/buffer/s_lock.c,v
retrieving revision 1.29
diff -c -r1.29 s_lock.c
*** src/backend/storage/buffer/s_lock.c 2001/01/14 05:08:15 1.29
--- src/backend/storage/buffer/s_lock.c 2001/01/19 20:28:20
***************
*** 115,123 ****
}
}
-
-
-
/*
* Various TAS implementations that cannot live in s_lock.h as no inline
* definition exists (yet).
--- 115,120 ----
***************
*** 136,153 ****
tas_dummy() /* really means: extern int tas(slock_t
* **lock); */
{
! __asm__(" \n\
! .global _tas \n\
! _tas: \n\
! movel sp@(0x4),a0 \n\
! tas a0@ \n\
! beq _success \n\
! moveq #-128,d0 \n\
! rts \n\
! _success: \n\
! moveq #0,d0 \n\
! rts \n\
! ");
}
#endif /* __m68k__ */
--- 133,150 ----
tas_dummy() /* really means: extern int tas(slock_t
* **lock); */
{
! __asm__ __volatile__(
! "\
! .global _tas \n\
! _tas: \n\
! movel sp@(0x4),a0 \n\
! tas a0@ \n\
! beq _success \n\
! moveq #-128,d0 \n\
! rts \n\
! _success: \n\
! moveq #0,d0 \n\
! rts");
}
#endif /* __m68k__ */
***************
*** 160,181 ****
static void
tas_dummy()
{
! __asm__(" \n\
! .globl tas \n\
! .globl _tas \n\
! _tas: \n\
! tas: \n\
! lwarx r5,0,r3 \n\
! cmpwi r5,0 \n\
! bne fail \n\
! addi r5,r5,1 \n\
! stwcx. r5,0,r3 \n\
! beq success \n\
! fail: li r3,1 \n\
! blr \n\
! success: \n\
! li r3,0 \n\
! blr \n\
");
}
--- 157,179 ----
static void
tas_dummy()
{
! __asm__ __volatile__(
! "\
! .globl tas \n\
! .globl _tas \n\
! _tas: \n\
! tas: \n\
! lwarx r5,0,r3 \n\
! cmpwi r5,0 \n\
! bne fail \n\
! addi r5,r5,1 \n\
! stwcx. r5,0,r3 \n\
! beq success \n\
! fail: li r3,1 \n\
! blr \n\
! success: \n\
! li r3,0 \n\
! blr \n\
");
}
***************
*** 186,206 ****
static void
tas_dummy()
{
! __asm__(" \n\
! .global tas \n\
! tas: \n\
! lwarx 5,0,3 \n\
! cmpwi 5,0 \n\
! bne fail \n\
! addi 5,5,1 \n\
! stwcx. 5,0,3 \n\
! beq success \n\
! fail: li 3,1 \n\
! blr \n\
! success: \n\
! li 3,0 \n\
! blr \n\
! ");
}
#endif /* __powerpc__ */
--- 184,204 ----
static void
tas_dummy()
{
! __asm__ __volatile__(
! "\
! .global tas \n\
! tas: \n\
! lwarx 5,0,3 \n\
! cmpwi 5,0 \n\
! bne fail \n\
! addi 5,5,1 \n\
! stwcx. 5,0,3 \n\
! beq success \n\
! fail: li 3,1 \n\
! blr \n\
! success: \n\
! li 3,0 \n\
! blr");
}
#endif /* __powerpc__ */
***************
*** 209,230 ****
static void
tas_dummy()
{
! __asm__(" \n\
! .global tas \n\
! tas: \n\
! .frame $sp, 0, $31 \n\
! ll $14, 0($4) \n\
! or $15, $14, 1 \n\
! sc $15, 0($4) \n\
! beq $15, 0, fail \n\
! bne $14, 0, fail \n\
! li $2, 0 \n\
! .livereg 0x2000FF0E,0x00000FFF \n\
! j $31 \n\
! fail: \n\
! li $2, 1 \n\
! j $31 \n\
! ");
}
#endif /* __mips__ */
--- 207,228 ----
static void
tas_dummy()
{
! __asm__ _volatile__(
! "\
! .global tas \n\
! tas: \n\
! .frame $sp, 0, $31 \n\
! ll $14, 0($4) \n\
! or $15, $14, 1 \n\
! sc $15, 0($4) \n\
! beq $15, 0, fail\n\
! bne $14, 0, fail\n\
! li $2, 0 \n\
! .livereg 0x2000FF0E,0x00000FFF \n\
! j $31 \n\
! fail: \n\
! li $2, 1 \n\
! j $31");
}
#endif /* __mips__ */
Index: src/include/storage/s_lock.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/storage/s_lock.h,v
retrieving revision 1.82
diff -c -r1.82 s_lock.h
*** src/include/storage/s_lock.h 2001/01/19 07:03:53 1.82
--- src/include/storage/s_lock.h 2001/01/19 20:28:21
***************
*** 103,111 ****
* Standard _asm format:
*
* __asm__ __volatile__(
! * "command;"
! * "command;"
! * "command;"
* : "=r"(_res) return value, in register
* : "r"(lock) argument, 'lock pointer', in register
* : "r0"); inline code uses this register
--- 103,111 ----
* Standard _asm format:
*
* __asm__ __volatile__(
! * "command \n"
! * "command \n"
! * "command \n"
* : "=r"(_res) return value, in register
* : "r"(lock) argument, 'lock pointer', in register
* : "r0"); inline code uses this register
***************
*** 121,128 ****
register slock_t _res = 1;
__asm__ __volatile__(
! "lock;"
! "xchgb %0,%1;"
: "=q"(_res), "=m"(*lock)
: "0"(_res));
return (int) _res;
--- 121,128 ----
register slock_t _res = 1;
__asm__ __volatile__(
! "lock \n"
! "xchgb %0,%1 \n"
: "=q"(_res), "=m"(*lock)
: "0"(_res));
return (int) _res;
***************
*** 140,146 ****
long int ret;
__asm__ __volatile__(
! "xchg4 %0=%1,%2;"
: "=r"(ret), "=m"(*lock)
: "r"(1), "1"(*lock)
: "memory");
--- 140,146 ----
long int ret;
__asm__ __volatile__(
! "xchg4 %0=%1,%2 \n"
: "=r"(ret), "=m"(*lock)
: "r"(1), "1"(*lock)
: "memory");
***************
*** 160,166 ****
register slock_t _res = 1;
__asm__ __volatile__(
! "swpb %0, %0, [%3];"
: "=r"(_res), "=m"(*lock)
: "0"(_res), "r"(lock));
return (int) _res;
--- 160,166 ----
register slock_t _res = 1;
__asm__ __volatile__(
! "swpb %0, %0, [%3] \n"
: "=r"(_res), "=m"(*lock)
: "0"(_res), "r"(lock));
return (int) _res;
***************
*** 180,190 ****
int _res;
__asm__ __volatile__(
! "la 1,1;"
! "l 2,%2;"
! "slr 0,0;"
! "cs 0,1,0(2);"
! "lr %1,0;"
: "=m"(lock), "=d"(_res)
: "m"(lock)
: "0", "1", "2");
--- 180,190 ----
int _res;
__asm__ __volatile__(
! "la 1,1 \n"
! "l 2,%2 \n"
! "slr 0,0 \n"
! "cs 0,1,0(2) \n"
! "lr %1,0 \n"
: "=m"(lock), "=d"(_res)
: "m"(lock)
: "0", "1", "2");
***************
*** 204,210 ****
register slock_t _res = 1;
__asm__ __volatile__(
! "ldstub [%2], %0;"
: "=r"(_res), "=m"(*lock)
: "r"(lock));
return (int) _res;
--- 204,210 ----
register slock_t _res = 1;
__asm__ __volatile__(
! "ldstub [%2], %0 \n"
: "=r"(_res), "=m"(*lock)
: "r"(lock));
return (int) _res;
***************
*** 222,229 ****
register int rv;
__asm__ __volatile__(
! "tas %1;"
! "sne %0;"
: "=d"(rv), "=m"(*lock)
: "1"(*lock)
: "cc");
--- 222,229 ----
register int rv;
__asm__ __volatile__(
! "tas %1 \n"
! "sne %0 \n"
: "=d"(rv), "=m"(*lock)
: "1"(*lock)
: "cc");
***************
*** 249,258 ****
register _res;
__asm__ __volatile__(
! "movl $1, r0;"
! "bbssi $0, (%1), 1f;"
! "clrl r0;"
! "1: movl r0, %0;"
: "=r"(_res)
: "r"(lock)
: "r0");
--- 249,258 ----
register _res;
__asm__ __volatile__(
! "movl $1, r0 \n"
! "bbssi $0, (%1), 1f \n"
! "clrl r0 \n"
! "1: movl r0, %0 \n"
: "=r"(_res)
: "r"(lock)
: "r0");
***************
*** 271,278 ****
register _res;
__asm__ __volatile__(
! "sbitb 0, %0;"
! "sfsd %1;"
: "=m"(*lock), "=r"(_res));
return (int) _res;
}
--- 271,278 ----
register _res;
__asm__ __volatile__(
! "sbitb 0, %0 \n"
! "sfsd %1 \n"
: "=m"(*lock), "=r"(_res));
return (int) _res;
}
***************
*** 339,354 ****
register slock_t _res;
__asm__ __volatile__(
! "ldq $0, %0;"
! "bne $0, 2f;"
! "ldq_l %1, %0;"
! "bne %1, 2f;"
! "mov 1, $0;"
! "stq_c $0, %0;"
! "beq $0, 2f;"
! "mb;"
! "br 3f;"
! "2: mov 1, %1;"
"3:"
: "=m"(*lock), "=r"(_res)
:
--- 339,354 ----
register slock_t _res;
__asm__ __volatile__(
! "ldq $0, %0 \n"
! "bne $0, 2f \n"
! "ldq_l %1, %0 \n"
! "bne %1, 2f \n"
! "mov 1, $0 \n"
! "stq_c $0, %0 \n"
! "beq $0, 2f \n"
! "mb \n"
! "br 3f \n"
! "2: mov 1, %1 \n"
"3:"
: "=m"(*lock), "=r"(_res)
: