From: Tom Lane <tgl@sss.pgh.pa.us>
To: YunQiang Su <wzssyqa@gmail.com>
cc: Andres Freund <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: 
 <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>
Comments: In-reply-to YunQiang Su <wzssyqa@gmail.com>
	message dated "Sat, 22 Jun 2019 12:21:09 +0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <19681.1561230332.1@sss.pgh.pa.us>
Date: Sat, 22 Jun 2019 15:05:32 -0400
Message-ID: <19682.1561230332@sss.pgh.pa.us>
Precedence: bulk

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