Re: [HACKERS] s_lock.h problem on S/Linux

I haven't seen any followups to this, but I finally got around to
compiling the system again myself, and David's fix is not quite right.
He says to apply this:

> *** src/include/storage/s_lock.h.orig Sun Jun 14 19:37:47 1998
> --- src/include/storage/s_lock.h Sat Jun 20 18:01:13 1998
> ***************
> *** 130,136 ****
>
> __asm__("ldstub [%1], %0" \
> : "=r"(_res), "=m"(*lock) \
> ! : "1"(lock));
> return (int) _res;
> }
> #endif /* sparc */
> --- 130,136 ----
>
> __asm__("ldstub [%1], %0" \
> : "=r"(_res), "=m"(*lock) \
> ! : "0"(_res));
> return (int) _res;
> }
> #endif /* sparc */

However, the reference to the lock pointer as "1" was closer to being
correct that then "0" is! The trouble is that the compiler doesn't
like the mixed indirection in the references for the lock pointer with
the "1" there in the original. Changing the input parameter as shown
to indicate _res fixes this, but is wrong, since that's not the input.
In the current sources, the "1" has been changed to a "0", erroneously
calling _res an input, but the name of the variable to use is still
'lock', making it really confusing by fetching the right input (the
pointer), and stuffing it into the wrong register -- and causing the
assembler to join in the chorus of complaints when it sees the double
dereferencing brackets in its source... :-)

Much better is to actually specify the constraints individually, and
then simply refer to the input parameter in the instruction. Here's
the patch I have to apply to the current sources to get it to compile
and work right (I've tested it with s_lock_test, of course):

Index: s_lock.h
===================================================================
RCS file: /usr/local/cvsroot/pgsql/src/include/storage/s_lock.h,v
retrieving revision 1.39
diff -r1.39 s_lock.h
131,133c131,133
< __asm__("ldstub [%1], %0" \
< : "=r"(_res), "=m"(*lock) \
< : "0"(lock));
---
> __asm__("ldstub [%2], %0"
> : "=r" (_res), "=m" (*lock)
> : "r" (lock));

Oh, yeah, I guess I didn't have to remove the backslashes, but this is
the GCC section, and they're not needed.

-tih
--
Popularity is the hallmark of mediocrity. --Niles Crane, "Frasier"