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 15:05:32 -0400
Message-ID: <19682.1561230332@sss.pgh.pa.us> (raw)
In-Reply-To: <CAKcpw6VZVerXTHWSxp341GeogHQBOY9p5w3VRwGMXcxcr_hhtw@mail.gmail.com>
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>
YunQiang Su <wzssyqa@gmail.com> writes:
> Since NetBSD set the default ISA as MIPS I, and then we use ".set mips2",
> in fact a bug.
> I think that we should use some non-asm code for MIPS I ( our own
> spinlock implementations?)
You probably won't be surprised to hear that we keep having this
discussion ;-). Sure, we could essentially throw away all of s_lock.h in
favor of relying on __sync_lock_test_and_set() and __sync_lock_release().
But doing that would essentially abdicate having control over, or even
having much knowledge about, infrastructure that's absolutely critical
to the correctness and performance of Postgres. And we've seen a bunch
of cases where the compiler builtins are just not very well implemented
for non-mainstream architectures.
I got debian-mips-under-qemu working thanks to the files you sent a link
to, and I was curious to see what that gcc version would generate for
__sync_lock_test_and_set() and __sync_lock_release(), if I asked for
MIPS-I code. That's not the default of course, but "gcc -march=mips1
-mabi=32" seems to work. And what I get is
.set push
.set mips2
1:
ll $2,0($3)
li $1,1
sc $1,0($3)
beq $1,$0,1b
nop
sync
.set pop
and
.set push
.set mips2
sync
.set pop
sw $0,0($3)
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. Any other choice would be completely disastrous
for performance on anything newer than MIPS-I ... and really, if you
are doing something that requires userland synchronization primitives,
you'd better not be using MIPS-I. It hasn't got them.
In short, moving to __sync_lock_test_and_set() would change basically
nothing on MIPS, but we'd no longer have any visibility into or control
over what it was doing. It would open us to other problems too -- for
example, if you try to apply __sync_lock_test_and_set() to a char
rather than an int, the code you get for that on MIPS is a whole lot
worse, but we'd have no visibility of that.
> Do you have any NetBSD image that I can have a test of new patch?
I've not been able to get NetBSD/MIPS to work under qemu. It does
mostly work under gxemul, but that has so many bugs as to be nigh
useless --- the float arithmetic emulation is what usually gets me
when I try to do anything with Postgres.
regards, tom lane
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: <19682.1561230332@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