Right, that's why I'm recommending adding a comment so we don't have someone 
flag this a third time :)

-Dave

On Mar 25, 2015, at 4:43 PM, George Bosilca <bosi...@icl.utk.edu> wrote:

> I had the same impression but them I went and read the Intel documentation 
> and xchg is one of these exceptions where the lock is implicit.
> 
>   George.
> 
> 
> On Wed, Mar 25, 2015 at 4:59 PM, Dave Goodell (dgoodell) <dgood...@cisco.com> 
> 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.
> 
> 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
> 
> _______________________________________________
> devel mailing list
> de...@open-mpi.org
> Subscription: http://www.open-mpi.org/mailman/listinfo.cgi/devel
> Link to this post: 
> http://www.open-mpi.org/community/lists/devel/2015/03/17153.php
> 
> _______________________________________________
> devel mailing list
> de...@open-mpi.org
> Subscription: http://www.open-mpi.org/mailman/listinfo.cgi/devel
> Link to this post: 
> http://www.open-mpi.org/community/lists/devel/2015/03/17154.php

Reply via email to