Re: BUG #15844: MIPS: remove .set mips2 in s_lock.h to fix r6 build

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Re: BUG #15844: MIPS: remove .set mips2 in s_lock.h to fix r6 build
Дата
Msg-id 27494.1561243027@sss.pgh.pa.us
обсуждение исходный текст
Ответ на Re: BUG #15844: MIPS: remove .set mips2 in s_lock.h to fix r6 build  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: BUG #15844: MIPS: remove .set mips2 in s_lock.h to fix r6 build  (YunQiang Su <wzssyqa@gmail.com>)
Список pgsql-bugs
I wrote:
> So the gcc guys aren't any more wedded than we are to the rule that
> one must not generate LL/SC on MIPS-I.  They're probably assuming the
> same thing we are, which is that if you're actually doing this on
> MIPS-I then you'll get an instruction trap and the kernel will
> emulate it for you.

Actually, after further contemplating that argument (which I was reminded
of by excavating in our commit log), I realize that having a .set mips2
in the ASM is actually the right way to do this, because that only results
in LL/SC getting emulated when we need them to be.  Forcing -march=mips2
across the entire Postgres build would exceed our authority really ---
yeah, it might produce better code quality, but it's not our business to
override platform-level target architecture decisions.

So I now think your original proposal was about the best way to do this,
though I'd tweak it to make the test be "#if __mips_isa_rev < 2", as
attached.  It doesn't seem like forcing the ISA level down is ever
something we want to do, even if we know there's no practical difference.

            regards, tom lane

diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index a9a92de..d2c707e 100644
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
@@ -598,13 +598,31 @@ tas(volatile slock_t *lock)
 
 
 #if defined(__mips__) && !defined(__sgi)    /* non-SGI MIPS */
-/* Note: R10000 processors require a separate SYNC */
 #define HAS_TEST_AND_SET
 
 typedef unsigned int slock_t;
 
 #define TAS(lock) tas(lock)
 
+/*
+ * Original MIPS-I processors lacked the LL/SC instructions, but if we are
+ * so unfortunate as to be running on one of those, we expect that the kernel
+ * will handle the illegal-instruction traps and emulate them for us.  On
+ * anything newer (and really, MIPS-I is extinct) LL/SC is the only sane
+ * choice because any other synchronization method must involve a kernel
+ * call.  Unfortunately, many toolchains still default to MIPS-I as the
+ * codegen target, so we have to be prepared to force the assembler to
+ * accept LL/SC.
+ *
+ * R10000 and up processors require a separate SYNC, which has the same
+ * issues as LL/SC.
+ */
+#if __mips_isa_rev < 2
+#define MIPS_SET_MIPS2    "       .set mips2          \n"
+#else
+#define MIPS_SET_MIPS2
+#endif
+
 static __inline__ int
 tas(volatile slock_t *lock)
 {
@@ -614,7 +632,7 @@ tas(volatile slock_t *lock)
 
     __asm__ __volatile__(
         "       .set push           \n"
-        "       .set mips2          \n"
+        MIPS_SET_MIPS2
         "       .set noreorder      \n"
         "       .set nomacro        \n"
         "       ll      %0, %2      \n"
@@ -636,7 +654,7 @@ do \
 { \
     __asm__ __volatile__( \
         "       .set push           \n" \
-        "       .set mips2          \n" \
+        MIPS_SET_MIPS2 \
         "       .set noreorder      \n" \
         "       .set nomacro        \n" \
         "       sync                \n" \

В списке pgsql-bugs по дате отправления:

Предыдущее
От: Tom Lane
Дата:
Сообщение: Re: BUG #15844: MIPS: remove .set mips2 in s_lock.h to fix r6 build
Следующее
От: YunQiang Su
Дата:
Сообщение: Re: BUG #15844: MIPS: remove .set mips2 in s_lock.h to fix r6 build