Re: [PATCH] Porting small OpenBSD changes. - Mailing list pgsql-hackers

David CARLIER <devnexen@gmail.com> writes:
> On 20 November 2017 at 18:49, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> OTOH, we still have M68K
>> and VAX stanzas in that file, so I suppose it's silly to complain
>> about 88K.  A bigger issue is that I wonder whether that code has
>> ever been tested: it does not look to me like the __asm__ call is
>> even syntactically correct.  There should be colons in it.

> True :-) corrected. Thanks.

I still dare to doubt whether you've tested this, because AFAICS
the operand numbering is wrong.  The "r"(lock) operand is number 3
given these operand declarations, not number 2.

Our practice elsewhere in s_lock.h is to use a "+" constraint instead of
duplicated operands, and I think that's better style because it avoids any
question of whether you're supposed to count duplicated operands.

Also, per the comment near s_lock.h line 115, it's important to specify
"+m"(*lock) as an output operand so that gcc knows that the asm
clobbers *lock.  It's possible that "memory" makes that redundant,
but I'd just as soon maintain consistency with the well-tested
other parts of the file.

So I propose the attached patch instead.  It would still be a good idea
to actually test this ;-)

            regards, tom lane

diff --git a/src/backend/storage/lmgr/s_lock.c b/src/backend/storage/lmgr/s_lock.c
index 40d8156..247d7b8 100644
--- a/src/backend/storage/lmgr/s_lock.c
+++ b/src/backend/storage/lmgr/s_lock.c
@@ -251,7 +251,7 @@ static void
 tas_dummy()
 {
     __asm__ __volatile__(
-#if defined(__NetBSD__) && defined(__ELF__)
+#if (defined(__NetBSD__) || defined(__OpenBSD__)) && defined(__ELF__)
 /* no underscore for label and % for registers */
                          "\
 .global        tas                 \n\
@@ -276,7 +276,7 @@ _tas:                            \n\
 _success:                        \n\
             moveq     #0,d0        \n\
             rts                    \n"
-#endif                            /* __NetBSD__ && __ELF__ */
+#endif                            /* (__NetBSD__ || __OpenBSD__) && __ELF__ */
         );
 }
 #endif                            /* __m68k__ && !__linux__ */
diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index 99e1098..35088db 100644
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
@@ -543,6 +543,30 @@ tas(volatile slock_t *lock)
 #endif     /* (__mc68000__ || __m68k__) && __linux__ */


+/* Motorola 88k */
+#if defined(__m88k__)
+#define HAS_TEST_AND_SET
+
+typedef unsigned int slock_t;
+
+#define TAS(lock) tas(lock)
+
+static __inline__ int
+tas(volatile slock_t *lock)
+{
+    register slock_t _res = 1;
+
+    __asm__ __volatile__(
+        "    xmem    %0, %2, %%r0    \n"
+:        "+r"(_res), "+m"(*lock)
+:        "r"(lock)
+:        "memory");
+    return (int) _res;
+}
+
+#endif     /* __m88k__ */
+
+
 /*
  * VAXen -- even multiprocessor ones
  * (thanks to Tom Ivar Helbekkmo)