Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

Subject: Re: [OMPI devel] Bug in openmpi-1.5/opal/config/opal_config_asm.m4
From: George Bosilca (bosilca_at_[hidden])
Date: 2011-02-23 17:03:12


Or how about this version ? Here I use the + modifier and I don't put any constraints on the input line.

static inline int32_t opal_atomic_add_32(volatile int32_t* v, int i)
{
   int ret = i;
  __asm__ __volatile__(
                       SMPLOCK "xaddl %1,%0"
                       : "+m" (*v), "+r" (ret)
                       :
                       : "memory", "cc"
                       );
  return (ret+i);
}

  george.

On Feb 23, 2011, at 16:59 , George Bosilca wrote:

> Jay,
>
> Thanks for the code. The code you pointed out is only used during configure, so I don't think is that critical. However, we use similar code deep into our voodoo assembly generation, for opal_atomic_add_32 and opal_atomic_sub_32.
>
> So if I understand your statement the correct version of the code should be
>
> static inline int32_t opal_atomic_add_32(volatile int32_t* v, int i)
> {
> int ret = i;
> __asm__ __volatile__(
> SMPLOCK "xaddl %1,%0"
> :"=m" (*v), "+r" (ret)
>>> new >> :"m" (*v)
> );
> return (ret+i);
> }
>
> On the GCC extended ASM documentation (http://gcc.gnu.org/onlinedocs/gcc/Modifiers.html#Modifiers), it is specified:
>
> `=' Means that this operand is write-only for this instruction: the previous value is discarded and replaced by output data.
>
> `+' Means that this operand is both read and written by the instruction.
>
> Based on this info, I would rather rewrite this function like this:
>
> static inline int32_t opal_atomic_add_32(volatile int32_t* v, int i)
> {
> int ret = i;
> __asm__ __volatile__(
> SMPLOCK "xaddl %1,%0"
> : "=m" (*v), "=r" (ret)
> : "m" (*v), "1" (ret)
> : "memory", "cc"
> );
> return (ret+i);
> }
>
> Can you ask the kindly GCC expert which version is "correct" (whatever the definition of correct might means related to GCC extended assembly). Do I have to specify = for the output if I put the register on the input? Aren't this conflicting?
>
> george.
>
> On Feb 23, 2011, at 13:04 , Jay Fenlason wrote:
>
>> I was recently handed
>> https://bugzilla.redhat.com/attachment.cgi?id=480307
>> for which a kindly GCC expert attached the enclosed patch. Apparently
>> this only causes problems on 32-bit i686 machines, which could by why
>> it has gone undetected until now.
>>
>> -- JF
>>
>> --- openmpi-1.5/opal/config/opal_config_asm.m4.jj 2010-09-28 23:33:51.000000000 +0200
>> +++ openmpi-1.5/opal/config/opal_config_asm.m4 2011-02-23 01:39:21.191433509 +0100
>> @@ -885,7 +885,7 @@ AC_DEFUN([OMPI_CONFIG_ASM],[
>> ompi_cv_asm_arch="AMD64"
>> fi
>> OPAL_ASM_SUPPORT_64BIT=1
>> - OMPI_GCC_INLINE_ASSIGN='"xaddl %1,%0" : "=m"(ret), "+r"(negone)'
>> + OMPI_GCC_INLINE_ASSIGN='"xaddl %1,%0" : "=m"(ret), "+r"(negone) : "m"(ret)'
>> ;;
>>
>> ia64-*)
>> _______________________________________________
>> devel mailing list
>> devel_at_[hidden]
>> http://www.open-mpi.org/mailman/listinfo.cgi/devel
>
> "I disapprove of what you say, but I will defend to the death your right to say it"
> -- Evelyn Beatrice Hall
>
>
> _______________________________________________
> devel mailing list
> devel_at_[hidden]
> http://www.open-mpi.org/mailman/listinfo.cgi/devel

"To preserve the freedom of the human mind then and freedom of the press, every spirit should be ready to devote itself to martyrdom; for as long as we may think as we will, and speak as we think, the condition of man will proceed in improvement."
  -- Thomas Jefferson, 1799