Re: [PATCHES] Reorganization of spinlock defines - Mailing list pgsql-hackers

Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > He is uncomfortable with the port/*.h changes at this point, so it seems
> > I am going to have to add Itanium/Opteron tests to most of those files.
>
> Why don't you try to put together a proposed patch of that kind, and
> then we can look to see how big and ugly it is compared to the other?
> If the alternative is shown to be really messy, that would sway my
> opinion, maybe Marc's too.

OK, here is an Opteron/Itanium patch that might work.  I say "might"
because I don't have a lot of confidence in the current spinlock
detection code.  There is an uncoupling between the definition of
HAS_TEST_AND_SET, the data type used by slock_t, and the assembler code.

For example, here is darwin.h:

    #if defined(__ppc__)
    #define HAS_TEST_AND_SET
    #endif

    #if defined(__ppc__)
    typedef unsigned int slock_t;

    #else
    typedef unsigned char slock_t;

    #endif

Does this say that Darwin on something other than PPC doesn't have
spinlocks?  Is that going to hit a spinlock define, or fall through?

Also, look at NEED_I386_TAS_ASM:  It is used only by SCO compilers,
though it is defined for all Intel platforms.  The s_lock.h gcc test
already tests __i386__.  It really doesn't do anything on non-SCO
compilers, and non-SCO compilers are better testing for i386 anyway.

Also, Solaris has just this:

    #define HAS_TEST_AND_SET
    typedef unsigned char slock_t;

How do they know what CPU is being used?  Both Sparc and i386 use
"unsigned char", so I guess it is OK, but I think you can see what I
mean when I say I don't have a lot of confidence in what we have now.

Let me also add that some slock_t typedef's didn't match the assembly
code.  For example, __alpha_ on netbsd.h had slock_t defined as
"unsigned long", while in linux.h it was "long int".  I assumed the
alpha was the correct one, but clearly they should be the same because
they use the same assembly code.

See what I mean.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: src/include/port/bsdi.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/port/bsdi.h,v
retrieving revision 1.8
diff -c -c -r1.8 bsdi.h
*** src/include/port/bsdi.h    4 Aug 2003 00:43:32 -0000    1.8
--- src/include/port/bsdi.h    12 Sep 2003 15:14:15 -0000
***************
*** 1,10 ****
! #if defined(__i386__)
  #define NEED_I386_TAS_ASM
  #endif
  #if defined(__sparc__)
  #define NEED_SPARC_TAS_ASM
  #endif

  #define HAS_TEST_AND_SET

- typedef unsigned char slock_t;
--- 1,14 ----
! #if defined(__i386__) || defined(__x86_64__)
  #define NEED_I386_TAS_ASM
+ typedef unsigned char slock_t;
+ #endif
+ #if defined(__ia64)
+ typedef unsigned int slock_t;
  #endif
  #if defined(__sparc__)
  #define NEED_SPARC_TAS_ASM
+ typedef unsigned char slock_t;
  #endif

  #define HAS_TEST_AND_SET

Index: src/include/port/freebsd.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/port/freebsd.h,v
retrieving revision 1.10
diff -c -c -r1.10 freebsd.h
*** src/include/port/freebsd.h    4 Aug 2003 00:43:32 -0000    1.10
--- src/include/port/freebsd.h    12 Sep 2003 15:14:15 -0000
***************
*** 1,7 ****
! #if defined(__i386__)
  #define NEED_I386_TAS_ASM
  #define HAS_TEST_AND_SET
  typedef unsigned char slock_t;
  #endif

  #if defined(__sparc__)
--- 1,12 ----
! #if defined(__i386__) || defined(__x86_64__)
  #define NEED_I386_TAS_ASM
  #define HAS_TEST_AND_SET
  typedef unsigned char slock_t;
+ #endif
+
+ #if defined(__ia64)
+ #define HAS_TEST_AND_SET
+ typedef unsigned int slock_t;
  #endif

  #if defined(__sparc__)
Index: src/include/port/netbsd.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/port/netbsd.h,v
retrieving revision 1.9
diff -c -c -r1.9 netbsd.h
*** src/include/port/netbsd.h    4 Aug 2003 00:43:32 -0000    1.9
--- src/include/port/netbsd.h    12 Sep 2003 15:14:15 -0000
***************
*** 1,7 ****
! #if defined(__i386__)
  #define NEED_I386_TAS_ASM
  #define HAS_TEST_AND_SET
  typedef unsigned char slock_t;
  #endif

  #if defined(__sparc__)
--- 1,12 ----
! #if defined(__i386__) || defined(__x86_64__)
  #define NEED_I386_TAS_ASM
  #define HAS_TEST_AND_SET
  typedef unsigned char slock_t;
+ #endif
+
+ #if defined(__ia64)
+ #define HAS_TEST_AND_SET
+ typedef unsigned int slock_t;
  #endif

  #if defined(__sparc__)
Index: src/include/port/openbsd.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/port/openbsd.h,v
retrieving revision 1.8
diff -c -c -r1.8 openbsd.h
*** src/include/port/openbsd.h    4 Aug 2003 00:43:32 -0000    1.8
--- src/include/port/openbsd.h    12 Sep 2003 15:14:15 -0000
***************
*** 1,7 ****
! #if defined(__i386__)
  #define NEED_I386_TAS_ASM
  #define HAS_TEST_AND_SET
  typedef unsigned char slock_t;
  #endif

  #if defined(__sparc__)
--- 1,12 ----
! #if defined(__i386__) || defined(__x86_64__)
  #define NEED_I386_TAS_ASM
  #define HAS_TEST_AND_SET
  typedef unsigned char slock_t;
+ #endif
+
+ #if defined(__ia64)
+ #define HAS_TEST_AND_SET
+ typedef unsigned int slock_t;
  #endif

  #if defined(__sparc__)
Index: src/include/port/sco.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/port/sco.h,v
retrieving revision 1.13
diff -c -c -r1.13 sco.h
*** src/include/port/sco.h    28 Oct 2001 06:26:08 -0000    1.13
--- src/include/port/sco.h    12 Sep 2003 15:14:15 -0000
***************
*** 6,12 ****
--- 6,18 ----

  #define USE_UNIVEL_CC

+ #if defined(__ia64)
+ #define HAS_TEST_AND_SET
+ typedef unsigned int slock_t;
+ #else
  typedef unsigned char slock_t;
+ #endif
+

  #ifndef            BIG_ENDIAN
  #define            BIG_ENDIAN        4321
Index: src/include/port/univel.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/port/univel.h,v
retrieving revision 1.17
diff -c -c -r1.17 univel.h
*** src/include/port/univel.h    28 Oct 2001 06:26:08 -0000    1.17
--- src/include/port/univel.h    12 Sep 2003 15:14:15 -0000
***************
*** 7,13 ****
--- 7,18 ----
   ***************************************/
  #define USE_UNIVEL_CC

+ #if defined(__ia64)
+ typedef unsigned int slock_t;
+ #else
  typedef unsigned char slock_t;
+ #endif
+

  #ifndef            BIG_ENDIAN
  #define            BIG_ENDIAN        4321
Index: src/include/port/unixware.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/port/unixware.h,v
retrieving revision 1.11
diff -c -c -r1.11 unixware.h
*** src/include/port/unixware.h    28 Oct 2001 06:26:08 -0000    1.11
--- src/include/port/unixware.h    12 Sep 2003 15:14:15 -0000
***************
*** 10,16 ****
--- 10,21 ----
   ***************************************/
  #define USE_UNIVEL_CC

+ #if defined(__ia64)
+ typedef unsigned int slock_t;
+ #else
  typedef unsigned char slock_t;
+ #endif
+

  #ifndef            BIG_ENDIAN
  #define            BIG_ENDIAN        4321