PERFORCE change 72450 for review

[Stephan Uphoff]

On Thu, 2005-03-03 at 16:52, John Baldwin wrote:
> 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 at 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.

The linux code wraps the target of the atomic operations in structures
to avoid aliasing problems.
I really, really would like to know exactly why the developers thought
that this is necessary.
So far I have had no luck finding a reference. 

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