Re: [OMPI devel] 1.5rc5 - warnings from Sun C 5.10

Tue, 31 Aug 2010 14:26:55 -0400

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 phhargr...@lbl.gov 

Future Technologies Group
HPC Research Department                   Tel: +1-510-495-2352
Lawrence Berkeley National Laboratory     Fax: +1-510-486-6900

_______________________________________________
devel mailing list

de...@open-mpi.org
http://www.open-mpi.org/mailman/listinfo.cgi/devel


_______________________________________________
devel mailing list

de...@open-mpi.org
http://www.open-mpi.org/mailman/listinfo.cgi/devel


_______________________________________________
devel mailing list
de...@open-mpi.org
http://www.open-mpi.org/mailman/listinfo.cgi/devel



_______________________________________________
devel mailing list
de...@open-mpi.org
http://www.open-mpi.org/mailman/listinfo.cgi/devel

Reply via email to