Re: [HACKERS] s_lock.h problem on S/Linux - Mailing list pgsql-hackers

From Tom Ivar Helbekkmo
Subject Re: [HACKERS] s_lock.h problem on S/Linux
Date
Msg-id 8690lq42uz.fsf@barsoom.Hamartun.Priv.NO
Whole thread Raw
In response to Re: [HACKERS] s_lock.h problem on S/Linux  (Bruce Momjian <maillist@candle.pha.pa.us>)
Responses Re: [HACKERS] s_lock.h problem on S/Linux
List pgsql-hackers
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"

pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: Having Patch (against v6.3.2)
Next
From: Bruce Momjian
Date:
Subject: Re: [HACKERS] s_lock.h problem on S/Linux