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.

Also, this should probably be "xchgl" instead of "xchg".

> +                      "=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.

> +                      "0" (newval), "m" (*addr) :
> +                      "memory");
> +    return oldval;
> +}

-Dave

Reply via email to