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] 1.5rc5 - warnings from Sun C 5.10
From: Rolf vandeVaart (rolf.vandevaart_at_[hidden])
Date: 2010-08-31 14:18:54


I created a new ticket for this issue. It is
https://svn.open-mpi.org/trac/ompi/ticket/2560

Please go there for future details on this issue. But to quickly
summarize, I would like to
go with George's first recommendation that uses the "addl" command as
the "xaddl"
creates a second set of problems with the Sun Studio compiler.

Rolf

On 08/25/10 13:10, George Bosilca wrote:
> Right, that was a pretty intense discussion. However, I don't think we talked about replacing the = by a +. The difference is that = means write and + means read&write. Here is the assembly output from gcc -O3:
>
> with output "=m" (*v) | with output "+m" (*v)
> and input "ir" (i), "m" (*v) | and input "r" (i)
> |
> _opal_atomic_add_32: | _opal_atomic_add_32:
> LFB5: | LFB5:
> pushq %rbp | pushq %rbp
> LCFI3: | LCFI3:
> movq %rsp, %rbp | movq %rsp, %rbp
> LCFI4: | LCFI4:
> movq %rdi, -8(%rbp) | movq %rdi, -8(%rbp)
> movl %esi, -12(%rbp) | movl %esi, -12(%rbp)
> movq -8(%rbp), %rcx | movq -8(%rbp), %rcx
> movl -12(%rbp), %edx | movl -12(%rbp), %edx
> movq -8(%rbp), %rax | movq -8(%rbp), %rax
> lock;addl %edx,(%rcx) | lock;addl %edx,(%rcx)
> movq -8(%rbp), %rax | movq -8(%rbp), %rax
> movl (%rax), %eax | movl (%rax), %eax
> leave | leave
> ret | ret
>
> It generates multiple loads as %ras is updated before the lock. Useless!
>
> Now, if we put on the output "=m"(*v) and skip the (*)v in the input arguments the code looks like this:
>
> LFB7:
> pushq %rbp
> LCFI0:
> movq %rsp, %rbp
> LCFI1:
> movl $0, -4(%rbp)
> leaq -4(%rbp), %rax
> movl $1, %edx
> lock
> addl %edx,(%rax)
> movl -4(%rbp), %eax
> leave
> ret
>
> Which is a LOT better. Not perfect as it still generate a load after the locked addl, but this is because we wanted to return the (*v).
>
> Thus the code should look at least like this
>
> static inline int32_t opal_atomic_add_32(volatile int32_t* v, int i)
> {
> __asm__ __volatile__(
> SMPLOCK "addl %1,%0"
> :"=m" (*v)
> :"r" (i));
> return (*v); /* should be an atomic operation */
> }
>
> Now, if what we want back from this function is the __REAL__ result of the atomic addition, then the code is wrong. Well, mostly wrong under heavy usage (i.e. multiple threads doing atomics on the same variable).
>
> Here is the opal_atomic_add_32 returning the correct result. This is similar to the atomic intrinsic called add_and_fetch.
>
> 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)
> );
> return ret+i;
> }
>
> george.
>
> On Aug 25, 2010, at 10:58 , Rolf vandeVaart wrote:
>
>
>> With respect to the warnings with atomic.h, we have been down this road before.
>> Here is the ticket with the background information.
>>
>> https://svn.open-mpi.org/trac/ompi/ticket/1500
>>
>> Eventually, we decided to just live with the warnings. However, I will take a look at George's two suggestions.
>>
>> Rolf
>>
>>
>>
>> On 08/24/10 21:28, George Bosilca wrote:
>>
>>> On Aug 24, 2010, at 20:40 , Paul H. Hargrove wrote:
>>>
>>>
>>>
>>>
>>>> "../../../openmpi-1.5rc5/opal/include/opal/sys/ia32/atomic.h", line 170: warning: impossible constraint for "%1" asm operand
>>>>
>>>>
>>>>
>>> __asm__ __volatile__(
>>> SMPLOCK "addl %1,%0"
>>> :"=m" (*v)
>>> :"ir" (i), "m" (*v));
>>>
>>> The problem seems to come from the "ir". Based on a Sun blog about the gcc style asm inlining support (
>>> http://blogs.sun.com/x86be/entry/gcc_style_asm_inlining_support
>>> ) it appears that i (any size integer immediate constraint) and r (any registers in rax, rbx, rcx, rdx, rbp, rsi, rdi, rsp, r8 - r15). As we don't only apply our atomics on immediate I think we should drop the "i".
>>>
>>>
>>>
>>>
>>>> "../../../openmpi-1.5rc5/opal/include/opal/sys/ia32/atomic.h", line 170: warning: parameter in inline asm statement unused: %2
>>>>
>>>>
>>>>
>>> This one is more trickier. Because of the %2 I suspect that the second (*v) on the inputs is not matched to the first (*v) on the outputs. While this might be significantly bad under some circumstances, in this case I think it can be safely ignored.
>>>
>>> However I would like to try the following asm code instead with the SUN compiler:
>>>
>>> __asm__ __volatile__(
>>> SMPLOCK "addl %1,%0"
>>> :"+m" (*v)
>>> :"r" (i));
>>>
>>> Thanks,
>>> george.
>>>
>>>
>>>
>>>
>>>
>>>> "../../../openmpi-1.5rc5/opal/include/opal/sys/ia32/atomic.h", line 187: warning: impossible constraint for "%1" asm operand
>>>> "../../../openmpi-1.5rc5/opal/include/opal/sys/ia32/atomic.h", line 187: warning: parameter in inline asm statement unused: %2
>>>>
>>>> ../../../../openmpi-1.5rc5/ompi/mpi/cxx/file.cc", line 145: Warning (Anachronism): Formal argument read_conversion_fn of type extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*) in call to MPI_Register_datarep(char*, extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*), extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*), extern "C" int(*)(ompi_datatype_t*,int*,void*), void*) is being passed int(*)(void*,ompi_datatype_t*,int,void*,long long,void*).
>>>> "../../../../openmpi-1.5rc5/ompi/mpi/cxx/file.cc", line 146: Warning (Anachronism): Formal argument write_conversion_fn of type extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*) in call to MPI_Register_datarep(char*, extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*), extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*), extern "C" int(*)(ompi_datatype_t*,int*,void*), void*) is being passed int(*)(void*,ompi_datatype_t*,int,void*,long long,void*).
>>>> "../../../../openmpi-1.5rc5/ompi/mpi/cxx/file.cc", line 147: Warning (Anachronism): Formal argument dtype_file_extent_fn of type extern "C" int(*)(ompi_datatype_t*,int*,void*) in call to MPI_Register_datarep(char*, extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*), extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*), extern "C" int(*)(ompi_datatype_t*,int*,void*), void*) is being passed int(*)(ompi_datatype_t*,int*,void*).
>>>> "../../../../openmpi-1.5rc5/ompi/mpi/cxx/file.cc", line 172: Warning (Anachronism): Formal argument write_conversion_fn of type extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*) in call to MPI_Register_datarep(char*, extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*), extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*), extern "C" int(*)(ompi_datatype_t*,int*,void*), void*) is being passed int(*)(void*,ompi_datatype_t*,int,void*,long long,void*).
>>>> "../../../../openmpi-1.5rc5/ompi/mpi/cxx/file.cc", line 173: Warning (Anachronism): Formal argument dtype_file_extent_fn of type extern "C" int(*)(ompi_datatype_t*,int*,void*) in call to MPI_Register_datarep(char*, extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*), extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*), extern "C" int(*)(ompi_datatype_t*,int*,void*), void*) is being passed int(*)(ompi_datatype_t*,int*,void*).
>>>> "../../../../openmpi-1.5rc5/ompi/mpi/cxx/file.cc", line 197: Warning (Anachronism): Formal argument read_conversion_fn of type extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*) in call to MPI_Register_datarep(char*, extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*), extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*), extern "C" int(*)(ompi_datatype_t*,int*,void*), void*) is being passed int(*)(void*,ompi_datatype_t*,int,void*,long long,void*).
>>>> "../../../../openmpi-1.5rc5/ompi/mpi/cxx/file.cc", line 199: Warning (Anachronism): Formal argument dtype_file_extent_fn of type extern "C" int(*)(ompi_datatype_t*,int*,void*) in call to MPI_Register_datarep(char*, extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*), extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*), extern "C" int(*)(ompi_datatype_t*,int*,void*), void*) is being passed int(*)(ompi_datatype_t*,int*,void*).
>>>> "../../../../openmpi-1.5rc5/ompi/mpi/cxx/file.cc", line 224: Warning (Anachronism): Formal argument dtype_file_extent_fn of type extern "C" int(*)(ompi_datatype_t*,int*,void*) in call to MPI_Register_datarep(char*, extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*), extern "C" int(*)(void*,ompi_datatype_t*,int,void*,long long,void*), extern "C" int(*)(ompi_datatype_t*,int*,void*), void*) is being passed int(*)(ompi_datatype_t*,int*,void*).
>>>> [Though line numbers differ very slightly]
>>>>
>>>>
>>>> NEW, not seen with 1.4.3rc1, warnings:
>>>>
>>>> Many instances of the following:
>>>>
>>>> "../../../../openmpi-1.5rc5/orte/mca/ess/ess.h", line 61: warning: attribute "noreturn" may not be applied to variable, ignored
>>>> "../../../../openmpi-1.5rc5/orte/mca/errmgr/errmgr.h", line 138: warning: attribute "noreturn" may not be applied to variable, ignored
>>>> [Due to applying __opal_attribute_noreturn__ to a function pointer]
>>>>
>>>> Single instances of the following:
>>>>
>>>> "../../../../../openmpi-1.5rc5/opal/mca/crs/none/crs_none_module.c", line 136: warning: statement not reached
>>>> "../../../../openmpi-1.5rc5/orte/mca/plm/base/plm_base_rsh_support.c", line 462: warning: implicit function declaration: rindex
>>>> "../../../../openmpi-1.5rc5/orte/mca/plm/base/plm_base_rsh_support.c", line 462: warning: improper pointer/integer combination: op "="
>>>> "../../../../openmpi-1.5rc5/orte/mca/plm/base/plm_base_rsh_support.c", line 565: warning: improper pointer/integer combination: op "="
>>>> "../../../../../openmpi-1.5rc5/orte/mca/rmcast/tcp/rmcast_tcp.c", line 982: warning: assignment type mismatch:
>>>> "../../../../../openmpi-1.5rc5/orte/mca/rmcast/tcp/rmcast_tcp.c", line 1023: warning: assignment type mismatch:
>>>> "../../../../../openmpi-1.5rc5/orte/mca/rmcast/udp/rmcast_udp.c", line 877: warning: assignment type mismatch:
>>>> "../../../../../openmpi-1.5rc5/orte/mca/rmcast/udp/rmcast_udp.c", line 918: warning: assignment type mismatch:
>>>> "../../../../openmpi-1.5rc5/orte/tools/orte-ps/orte-ps.c", line 288: warning: initializer does not fit or is out of range: 0xfffffffe
>>>> "../../../../openmpi-1.5rc5/orte/tools/orte-ps/orte-ps.c", line 289: warning: initializer does not fit or is out of range: 0xfffffffe
>>>> "
>>>> "../../../../../openmpi-1.5rc5/ompi/mca/common/sm/common_sm_mmap.c", line 110: warning: improper pointer/integer combination: arg #3
>>>> "../../../../../openmpi-1.5rc5/ompi/mca/common/sm/common_sm_mmap.c", line 135: warning: improper pointer/integer combination: arg #3
>>>> "../../../../../openmpi-1.5rc5/ompi/mca/common/sm/common_sm_mmap.c", line 201: warning: assignment type mismatch:
>>>> pointer to char "=" pointer to int
>>>> "../../../../../openmpi-1.5rc5/ompi/mca/common/sm/common_sm_mmap.c", line 207: warning: assignment type mismatch:
>>>> pointer to char "=" pointer to int
>>>> "../../../../../openmpi-1.5rc5/ompi/mca/common/sm/common_sm_mmap.c", line 280: warning: argument #1 is incompatible with prototype:
>>>> prototype: pointer to char : "/usr/include/sys/mman.h", line 238
>>>> argument : pointer to struct mca_common_sm_mmap_t {struct opal_list_item_t {..} map_item, pointer to struct mca_common_sm_file_header_t {..} map_seg, pointer to unsigned char map_addr, pointer to unsigned char data_addr, unsigned int map_size, array[1025] of char map_path}
>>>>
>>>>
>>>> -Paul
>>>>
>>>> --
>>>> Paul H. Hargrove
>>>> PHHargrove_at_[hidden]
>>>>
>>>> Future Technologies Group
>>>> HPC Research Department Tel: +1-510-495-2352
>>>> Lawrence Berkeley National Laboratory Fax: +1-510-486-6900
>>>>
>>>> _______________________________________________
>>>> devel mailing list
>>>>
>>>> devel_at_[hidden]
>>>> http://www.open-mpi.org/mailman/listinfo.cgi/devel
>>>>
>>>>
>>>>
>>>>
>>> _______________________________________________
>>> devel mailing list
>>>
>>> devel_at_[hidden]
>>> http://www.open-mpi.org/mailman/listinfo.cgi/devel
>>>
>>>
>>>
>>>
>> _______________________________________________
>> devel mailing list
>> devel_at_[hidden]
>> http://www.open-mpi.org/mailman/listinfo.cgi/devel
>>
>
>
> _______________________________________________
> devel mailing list
> devel_at_[hidden]
> http://www.open-mpi.org/mailman/listinfo.cgi/devel
>