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

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