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,0stb    r4,45(r25)st    r23,48(r25)cal    r5,0(r0)stx    r23,r28,r27
        <----- spinlock releasebc    BO_IF_NOT,CR0_EQ,__L834st    r25,12(r26)                <----- store into
lock->headst   r25,16(r26)                <----- store into lock->taill    r4,12(r25)bl    .IpcSemaphoreLock{PR}
 

With "volatile" added in S_UNLOCK:
stb    r24,44(r25)stb    r3,45(r25)cmpi    0,r0,0cal    r5,0(r0)st    r23,48(r25)bc    BO_IF_NOT,CR0_EQ,__L81cst
r25,12(r26)               <----- store into lock->headstx    r23,r28,r27                <----- spinlock releasel
r3,8(r25)st   r25,16(r26)                <----- store into lock->taill    r4,12(r25)bl    .IpcSemaphoreLock{PR}
 

With "volatile" lock pointer in LWLockAcquire:
stb    r25,44(r23)stb    r3,45(r23)cmpi    0,r0,0cal    r5,0(r0)st    r24,48(r23)bc    BO_IF_NOT,CR0_EQ,__L850st
r23,12(r26)               <----- store into lock->headst    r23,16(r26)                <----- store into lock->tailstx
 r24,r28,r27                <----- spinlock releasel    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