Re: bad register name `%sil'

[YONETANI Tomokazu <qhwt+dragonfly-bugs@xxxxxxxxxx>]

On Mon, Feb 02, 2004 at 02:00:14AM +0900, YONETANI Tomokazu wrote:
> Is our gcc3 known to produce a bad code when '-O -march=pentium4'
> are specified? This is from buildkernel(sorry, very long lines):

This is reproducible even with CVS tip of gcc(3.5.0 20040206).
I've been reading FreeBSD-CURRENT's commit logs, and found something
relavant:

%%%
RCS file: /home/source/freebsd/cvs/src/sys/i386/include/atomic.h,v
Working file: atomic.h
head: 1.31
branch:
locks: strict
access list:
keyword substitution: kv
total revisions: 35;    selected revisions: 2
description:
----------------------------
revision 1.24
date: 2001/12/18 08:51:34;  author: jhb;  state: Exp;  lines: +23 -23
Allow the ATOMIC_ASM() macro to pass in the constraints on the V parameter
since the char versions need to use either ax, bx, cx, or dx.

Submitted by:   Peter Jeremy (mostly)
Recommended by: bde
----------------------------
revision 1.22
date: 2001/11/12 16:57:33;  author: jhb;  state: Exp;  lines: +22 -22
Use newer constraints for inline assembly for an operand that is both an
input and an output by using the '+' modifier rather than listing the
operand in both the input and output sections.

Reviwed by:     bde
%%%

Attached patch brings these changes into our version of atomic.h,
and -march=pentium4 doesn't fail on atomic_*() functions any more.

I've confirmed that this patch doesn't break buildworld/buildkernel
with gcc2 by doing
  __MAKE_CONF=/dev/null make -j3 buildworld KERNCONF=LINT buildkernel
on both old(without atomic.h change) and new(with atomic.h change)
kernels. Unfortunately I couldn't finish buildworld/buildkernel with
gcc3 because of breakages in other places.

By the way, I have some questions.
- do we keep macros for older version of gcc (gcc <= 2.8 version)?

- _nonlocked version of atomic_*() has `MPLOCKED' in front of OP,
  making it exactly the same with the version without `_nonlocked' suffix.
  is this intended? (the patch does not remove this)

static __inline void                                    \
atomic_##NAME##_##TYPE##_nonlocked(volatile u_##TYPE *p, u_##TYPE v)\
{                                                       \
        __asm __volatile(MPLOCKED OP                    \
                         : "=m" (*p)                    \
                         :  "0" (*p), "ir" (V));        \
}

Regards.


Index: sys/i386/include/atomic.h
===================================================================
RCS file: /home/source/dragonfly/cvs/src/sys/i386/include/atomic.h,v
retrieving revision 1.4
diff -u -r1.4 atomic.h
--- sys/i386/include/atomic.h	20 Jul 2003 03:55:25 -0000	1.4
+++ sys/i386/include/atomic.h	7 Feb 2004 12:03:12 -0000
@@ -63,7 +63,7 @@
  * This allows kernel modules to be portable between UP and SMP systems.
  */
 #if defined(KLD_MODULE)
-#define ATOMIC_ASM(NAME, TYPE, OP, V)			\
+#define ATOMIC_ASM(NAME, TYPE, OP, CONS, V)			\
 	extern void atomic_##NAME##_##TYPE(volatile u_##TYPE *p, u_##TYPE v); \
 	extern void atomic_##NAME##_##TYPE##_nonlocked(volatile u_##TYPE *p, u_##TYPE v);
 
@@ -84,20 +84,20 @@
 #if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ > 9)
 
 /* egcs 1.1.2+ version */
-#define ATOMIC_ASM(NAME, TYPE, OP, V)			\
+#define ATOMIC_ASM(NAME, TYPE, OP, CONS, V)		\
 static __inline void					\
 atomic_##NAME##_##TYPE(volatile u_##TYPE *p, u_##TYPE v)\
 {							\
 	__asm __volatile(MPLOCKED OP			\
-			 : "=m" (*p)			\
-			 :  "0" (*p), "ir" (V)); 	\
+			 : "+m" (*p)			\
+			 : CONS (V));			\
 }							\
 static __inline void					\
 atomic_##NAME##_##TYPE##_nonlocked(volatile u_##TYPE *p, u_##TYPE v)\
 {							\
 	__asm __volatile(MPLOCKED OP			\
-			 : "=m" (*p)			\
-			 :  "0" (*p), "ir" (V)); 	\
+			 : "+m" (*p)			\
+			 : CONS (V)); 			\
 }
 
 #else
@@ -124,25 +124,25 @@
 #if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ > 9)
 
 /* egcs 1.1.2+ version */
-ATOMIC_ASM(set,	     char,  "orb %b2,%0",   v)
-ATOMIC_ASM(clear,    char,  "andb %b2,%0", ~v)
-ATOMIC_ASM(add,	     char,  "addb %b2,%0",  v)
-ATOMIC_ASM(subtract, char,  "subb %b2,%0",  v)
-
-ATOMIC_ASM(set,	     short, "orw %w2,%0",   v)
-ATOMIC_ASM(clear,    short, "andw %w2,%0", ~v)
-ATOMIC_ASM(add,	     short, "addw %w2,%0",  v)
-ATOMIC_ASM(subtract, short, "subw %w2,%0",  v)
-
-ATOMIC_ASM(set,	     int,   "orl %2,%0",   v)
-ATOMIC_ASM(clear,    int,   "andl %2,%0", ~v)
-ATOMIC_ASM(add,	     int,   "addl %2,%0",  v)
-ATOMIC_ASM(subtract, int,   "subl %2,%0",  v)
-
-ATOMIC_ASM(set,	     long,  "orl %2,%0",   v)
-ATOMIC_ASM(clear,    long,  "andl %2,%0", ~v)
-ATOMIC_ASM(add,	     long,  "addl %2,%0",  v)
-ATOMIC_ASM(subtract, long,  "subl %2,%0",  v)
+ATOMIC_ASM(set,	     char,  "orb %b1,%0",  "iq", v)
+ATOMIC_ASM(clear,    char,  "andb %b1,%0", "iq",~v)
+ATOMIC_ASM(add,	     char,  "addb %b1,%0", "iq", v)
+ATOMIC_ASM(subtract, char,  "subb %b1,%0", "iq", v)
+
+ATOMIC_ASM(set,	     short, "orw %w1,%0",  "ir", v)
+ATOMIC_ASM(clear,    short, "andw %w1,%0", "ir",~v)
+ATOMIC_ASM(add,	     short, "addw %w1,%0", "ir", v)
+ATOMIC_ASM(subtract, short, "subw %w1,%0", "ir", v)
+
+ATOMIC_ASM(set,	     int,   "orl %1,%0",   "ir", v)
+ATOMIC_ASM(clear,    int,   "andl %1,%0",  "ir",~v)
+ATOMIC_ASM(add,	     int,   "addl %1,%0",  "ir", v)
+ATOMIC_ASM(subtract, int,   "subl %1,%0",  "ir", v)
+
+ATOMIC_ASM(set,	     long,  "orl %1,%0",   "ir", v)
+ATOMIC_ASM(clear,    long,  "andl %1,%0",  "ir",~v)
+ATOMIC_ASM(add,	     long,  "addl %1,%0",  "ir", v)
+ATOMIC_ASM(subtract, long,  "subl %1,%0",  "ir", v)
 
 #else