Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 3 Mar 2005 16:52:47 -0500
From:      John Baldwin <jhb@FreeBSD.org>
To:        Alan Cox <alc@cs.rice.edu>
Cc:        Perforce Change Reviews <perforce@FreeBSD.org>
Subject:   Re: PERFORCE change 72450 for review
Message-ID:  <200503031652.47596.jhb@FreeBSD.org>
In-Reply-To: <20050303212558.GA16936@cs.rice.edu>
References:  <200503032104.j23L4Pjw010114@repoman.freebsd.org> <20050303212558.GA16936@cs.rice.edu>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday 03 March 2005 04:25 pm, Alan Cox wrote:
> On Thu, Mar 03, 2005 at 09:04:25PM +0000, John Baldwin wrote:
> > http://perforce.freebsd.org/chv.cgi?CH=72450
> >
> > Change 72450 by jhb@jhb_slimer on 2005/03/03 21:03:35
> >
> > 	Clobber memory for cas{x,}a() inlines.
> >
> > 	Suggested by:  alc
> >
> > Affected files ...
> >
> > .. //depot/projects/smpng/sys/sparc64/include/cpufunc.h#20 edit
> >
> > Differences ...
> >
> > ==== //depot/projects/smpng/sys/sparc64/include/cpufunc.h#20 (text+ko)
> > ====
> >
> > @@ -63,14 +63,14 @@
> >  #define	casa(rs1, rs2, rd, asi) ({					\
> >  	u_int __rd = (uint32_t)(rd);					\
> >  	__asm __volatile("casa [%1] %2, %3, %0"				\
> > -	    : "+r" (__rd) : "r" (rs1), "n" (asi), "r" (rs2));		\
> > +	    : "+r" (__rd) : "r" (rs1), "n" (asi), "r" (rs2) : "memory");\
> >  	__rd;								\
> >  })
> >
> >  #define	casxa(rs1, rs2, rd, asi) ({					\
> >  	u_long __rd = (uint64_t)(rd);					\
> >  	__asm __volatile("casxa [%1] %2, %3, %0"			\
> > -	    : "+r" (__rd) : "r" (rs1), "n" (asi), "r" (rs2));		\
> > +	    : "+r" (__rd) : "r" (rs1), "n" (asi), "r" (rs2) : "memory");\
> >  	__rd;								\
> >  })
>
> The other, arguably "more correct", option is to declare the memory
> location referenced by rs1 as an input and output operand, like so
> from i386:  (I say "more correct" because the true operand here is the
> memory location referenced by rs1 not rs1 the register.)

Ah, yes, I can try that.

> static __inline pt_entry_t
> pte_load_store(pt_entry_t *ptep, pt_entry_t pte)
> {
> 	pt_entry_t r;
>
> 	__asm __volatile(
> 	    "xchgl %0,%1"
>
> 	    : "=m" (*ptep),
>
> 	      "=r" (r)
>
> 	    : "1" (pte),
>
> 	      "m" (*ptep));
> 	return (r);
> }
>
> (Note: this example does not use "+m" as an output constraint because
> Tor convinced me a few months ago that the gcc docs prohibit that: "+"
> is only to be used with registers.)

Hmm, this is what gcc info page says:

`+'
     Means that this operand is both read and written by the
     instruction.

     When the compiler fixes up the operands to satisfy the constraints,
     it needs to know which operands are inputs to the instruction and
     which are outputs from it.  `=' identifies an output; `+'
     identifies an operand that is both input and output; all other
     operands are assumed to be input only.

     If you specify `=' or `+' in a constraint, you put it in the first
     character of the constraint string.

It does say that '&' can't be used with a memory address it seems:

`&'
     Means (in a particular alternative) that this operand is an
     "earlyclobber" operand, which is modified before the instruction is
     finished using the input operands.  Therefore, this operand may
     not lie in a register that is used as an input operand or as part
     of any memory address.

This is important to sort out as the i386 atomic ops use '+m' rather 
extensively.

> Returning to the sparc, I'm not sure what the right constraint is for
> cas{x,}a's rs1.  I don't believe that "m" is appropriate because this
> particular instruction doesn't allow the destination to be a register
> plus an constant offset.  Perhaps, "V"?
>
> That said, we should add a "memory" clobber to the sparc64 atomic ops
> that include a memory barrier, particularly, the acquires.

Agreed.

> Regards,
> Alan

-- 
John Baldwin <jhb@FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200503031652.47596.jhb>