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

YunQiang Su <wzssyqa(at)gmail(dot)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