Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |  

This web mail archive is frozen.

This page is part of a frozen web archive of this mailing list.

You can still navigate around this archive, but know that no new mails have been added to it since July of 2016.

Click here to be taken to the new web archives of this list; it includes all the mails that are in this frozen archive plus all new mails that have been sent to the list since it was migrated to the new archives.

Subject: Re: [OMPI devel] Bug in openmpi-1.5/opal/config/opal_config_asm.m4
From: George Bosilca (bosilca_at_[hidden])
Date: 2011-02-24 01:56:09


Going on the Redhat thread regarding this issue (https://bugzilla.redhat.com/show_bug.cgi?id=679489), one can find the following comment

> Jakub Jelinek 2011-02-22 19:53:22 EST
> Could be "+m" (ret) too, but I think the "=m" + "m" variant should cover even
> prehistoric buggy gccs while "+m" might not work well there.

Apparently, for historical reasons the code should have =m on the output and m on the input. I could live with that. I'll take a look at our assembly to fix this.

  george.

On Feb 23, 2011, at 17:03 , George Bosilca wrote:

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