From: Tom Lane <tgl@sss.pgh.pa.us>
To: YunQiang Su <wzssyqa@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15844: MIPS: remove .set mips2 in s_lock.h to fix r6 build
Date: Sat, 22 Jun 2019 18:37:07 -0400
Message-ID: <27494.1561243027@sss.pgh.pa.us> (raw)
In-Reply-To: <19682.1561230332@sss.pgh.pa.us>
References: <15844-8f62fe7e163939b3@postgresql.org>
<15052.1560455595@sss.pgh.pa.us>
<CAKcpw6W2WBdTVnJSeV845V78bwOPDjhROfteX7D2o0TTU2_opA@mail.gmail.com>
<23287.1560655935@sss.pgh.pa.us>
<CAKcpw6W8Y56cxKJna6EsA+j2ZQ1p6DeXwMJFOOjb=baPigbV+Q@mail.gmail.com>
<25321.1560659282@sss.pgh.pa.us>
<CAKcpw6XYkD9Ku-0Fw6VZ6=wR030c5xCBOnTGsjLzFqdffkM8dw@mail.gmail.com>
<22615.1560701813@sss.pgh.pa.us>
<20190617163207.kgi5m4v2g2m252t5@alap3.anarazel.de>
<8749.1560790222@sss.pgh.pa.us>
<CAKcpw6VZVerXTHWSxp341GeogHQBOY9p5w3VRwGMXcxcr_hhtw@mail.gmail.com>
<19682.1561230332@sss.pgh.pa.us>
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
Attachments:avoid-asm-set-mips2-2.patch (text/x-diff, 1.7K) downloaddiff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.hindex 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" \
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, wzssyqa@gmail.com, andres@anarazel.de, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15844: MIPS: remove .set mips2 in s_lock.h to fix r6 build
In-Reply-To: <27494.1561243027@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox