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

Patch applied.


> 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"
> 


-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)