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
>>> de...@open-mpi.org
>>> 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
>> de...@open-mpi.org
>> 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
> de...@open-mpi.org
> 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


Reply via email to