Thread: s_lock.h problem on S/Linux
Hi hackers. I am having problems with the new spinlock code on the SPARC linux platform. (Latest CVSup) The compiler doesn't seem to like the "asm" part of s_lock.h for (sparc) Here's one of the compiles that fails. make[3]: Entering directory `/usr/local/pgsql/src/backend/storage/ipc' gcc -I../../../include -I../../../backend -O2 -Wall -Wmissing-prototypes -I../.. -c spin.c -o spin.o ../../../include/storage/s_lock.h: In function `SpinAcquire': ../../../include/storage/s_lock.h:131: inconsistent operand constraints in an `asm' ../../../include/storage/s_lock.h:131: inconsistent operand constraints in an `asm' ../../../include/storage/s_lock.h:131: inconsistent operand constraints in an `asm' ../../../include/storage/s_lock.h:131: inconsistent operand constraints in an `asm' ../../../include/storage/s_lock.h: In function `SpinRelease': ../../../include/storage/s_lock.h:131: inconsistent operand constraints in an `asm' ../../../include/storage/s_lock.h:131: inconsistent operand constraints in an `asm' make[3]: *** [spin.o] Error 1 Th ccsym information may be useful. [postgres@sparclinux pgsql]$ src/tools/ccsym __GNUC__=2 __GNUC_MINOR__=7 __ELF__ unix sparc linux __ELF__ __unix__ __sparc__ __linux__ __unix __sparc __linux system=unix system=posix cpu=sparc machine=sparc [postgres@sparclinux pgsql]$ Keith.
>
> Hi hackers.
>
> I am having problems with the new spinlock code on the SPARC linux
> platform. (Latest CVSup)
>
> The compiler doesn't seem to like the "asm" part of s_lock.h for (sparc)
>
> Here's one of the compiles that fails.
Thank you for testing and reporting this. It is my fault of course, but as
I don't have access to a sparc for testing I just did what I could. I am
guessing here, but please apply the following to your pgsql and let me know
what happens. Also, cd to src/storage/buffer and do 'make s_lock_test' as
well.
*** 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 */
-dg
David Gould dg@illustra.com 510.628.3783 or 510.305.9468
Informix Software (No, really) 300 Lakeside Drive Oakland, CA 94612
"Don't worry about people stealing your ideas. If your ideas are any
good, you'll have to ram them down people's throats." -- Howard Aiken
Patch applied.
> >
> > Hi hackers.
> >
> > I am having problems with the new spinlock code on the SPARC linux
> > platform. (Latest CVSup)
> >
> > The compiler doesn't seem to like the "asm" part of s_lock.h for (sparc)
> >
> > Here's one of the compiles that fails.
>
> Thank you for testing and reporting this. It is my fault of course, but as
> I don't have access to a sparc for testing I just did what I could. I am
> guessing here, but please apply the following to your pgsql and let me know
> what happens. Also, cd to src/storage/buffer and do 'make s_lock_test' as
> well.
>
>
> *** 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 */
>
> -dg
>
> David Gould dg@illustra.com 510.628.3783 or 510.305.9468
> Informix Software (No, really) 300 Lakeside Drive Oakland, CA 94612
> "Don't worry about people stealing your ideas. If your ideas are any
> good, you'll have to ram them down people's throats." -- Howard Aiken
>
>
--
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)
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"
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)