Re: Intermediate report for AIX 5L port

I said:
> Now that I think a little more, I am worried about the idea that the
> same thing could potentially happen in other modules that access shared
> memory and use spinlocks to serialize the access. Perhaps the fix I
> applied was wrong, and the correct fix is to change S_LOCK from
> #define S_UNLOCK(lock) (*(lock) = 0)
> to
> #define S_UNLOCK(lock) (*((volatile slock_t *) (lock)) = 0)

I have applied this patch also, since on reflection it seems the clearly
Right Thing. However, I find that AIX 5's compiler must have the LWLock
pointers marked volatile as well, else it still generates bad code.

Original assembly code fragment (approximately lines 244-271 of
lwlock.c):

l r3,8(r25)
stb r24,44(r25)
cmpi 0,r0,0
stb r4,45(r25)
st r23,48(r25)
cal r5,0(r0)
stx r23,r28,r27 <----- spinlock release
bc BO_IF_NOT,CR0_EQ,__L834
st r25,12(r26) <----- store into lock->head
st r25,16(r26) <----- store into lock->tail
l r4,12(r25)
bl .IpcSemaphoreLock{PR}

With "volatile" added in S_UNLOCK:

stb r24,44(r25)
stb r3,45(r25)
cmpi 0,r0,0
cal r5,0(r0)
st r23,48(r25)
bc BO_IF_NOT,CR0_EQ,__L81c
st r25,12(r26) <----- store into lock->head
stx r23,r28,r27 <----- spinlock release
l r3,8(r25)
st r25,16(r26) <----- store into lock->tail
l r4,12(r25)
bl .IpcSemaphoreLock{PR}

With "volatile" lock pointer in LWLockAcquire:

stb r25,44(r23)
stb r3,45(r23)
cmpi 0,r0,0
cal r5,0(r0)
st r24,48(r23)
bc BO_IF_NOT,CR0_EQ,__L850
st r23,12(r26) <----- store into lock->head
st r23,16(r26) <----- store into lock->tail
stx r24,r28,r27 <----- spinlock release
l r3,8(r23)
l r4,12(r23)
bl .IpcSemaphoreLock{PR}

I believe the second of these cases is inarguably a compiler bug.
It is moving a store (into lock->tail) across a store through a
volatile-qualified pointer. As I read the spec, that's not kosher.

regards, tom lane