Re: BUG #15844: MIPS: remove .set mips2 in s_lock.h to fix r6 build - Mailing list pgsql-bugs
| From | Tom Lane |
|---|---|
| Subject | Re: BUG #15844: MIPS: remove .set mips2 in s_lock.h to fix r6 build |
| Date | |
| Msg-id | 27494.1561243027@sss.pgh.pa.us Whole thread Raw |
| In response to | Re: BUG #15844: MIPS: remove .set mips2 in s_lock.h to fix r6 build (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
Re: BUG #15844: MIPS: remove .set mips2 in s_lock.h to fix r6 build
|
| List | 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 by date: