On Wed, Mar 25, 2015 at 08:59:31PM +0000, Dave Goodell (dgoodell) wrote:
> On Mar 25, 2015, at 3:02 PM, git...@crest.iu.edu wrote:
>
> > +static inline int32_t opal_atomic_swap_32( volatile int32_t *addr,
> > + int32_t newval)
> > +{
> > + int32_t oldval;
> > +
> > + __asm__ __volatile__("xchg %1, %0" :
>
> This code *looks* buggy because it lacks the "SMPLOCK" prefix, but can be
> safely omitted because "xchg" is always locked. A comment to this effect
> should be added.Sure. I will add comments to both the amd64 and ia32 atomics. > Also, this should probably be "xchgl" instead of "xchg". Agreed, xchngl would be consistent with the nmemonics used elsewhere in the ia32 atomics. > > + "=r" (oldval), "=m" (*addr) : > > Shouldn't the modifier on the second constraint above be "+" for the same > reasons as the rest of this commit? In that case I also think you can omit > the second constraint below altogether, though I'm less sure about that. That is indeed the case. The code was C&Ped from the amd64 atomics so I left it as is. I will probably go through and cleanup all of the atomics that have both "=m" "m". -Nathan
pgp8gElNUCD0q.pgp
Description: PGP signature
