1
1
openmpi/ompi/mpi/fortran/mpif-h/cart_create_f.c
Mark Allen 0a7f1e3cc5 in-place conversion macro writes into INPUT argument
In fint_2_int.h there are some conversion macros for logicals. It has
one path for OMPI_SIZEOF_FORTRAN_LOGICAL != SIZEOF_INT where a new array
would be allocated and the conversions then might expand to
    c_array[i] = (array[i] == 0 ? 0 : 1)
and another path for OMPI_SIZEOF_FORTRAN_LOGICAL == SIZEOF_INT where it
does things "in place", so the same conversion there would just be
    array[i] = (array[i] == 0 ? 0 : 1)

The problem is some of the logical arrays being converted are INPUT
arguments. And it's possible for some compilers to even put the argument
in read-only memory so the above "in place" conversion SEGV's.  A
testcase I have used
    call MPI_CART_SUB(oldcomm, (/.true.,.false./), newcomm, ierr)
and gfortran put the second arg in read-only mem.

In cart_sub_f.c you can trace the ompi_fortran_logical_t *remain_dims arg.
remain_dims[] is for input only, but the file uses
    OMPI_LOGICAL_ARRAY_NAME_DECL(remain_dims);
    OMPI_ARRAY_LOGICAL_2_INT(remain_dims, ndims);
    PMPI_Cart_sub(..., OMPI_LOGICAL_ARRAY_NAME_CONVERT(remain_dims), ...);
    OMPI_ARRAY_INT_2_LOGICAL(remain_dims, ndims);
to convert it to c-ints make a C call then restore it to Fortran logicals
before returning.

It's not always wrong to convert purely in-place, eg cart_get_f.c has
a periods[] that's exclusively for OUTPUT and it would be fine with the
macros as they were. But I still say the macros are invalid because they
don't distinguish whether they're being used on INPUT or OUTPUT args and
thus they can't be used in a way that's legal for both cases.

It might be possible to fix the macros by adding more of them so that
cart_create_f.c and cart_get_f.c would use different macros that give
more context. But my fix here is just to turn off the first block and
make all paths run as if OMPI_SIZEOF_FORTRAN_LOGICAL != SIZEOF_INT.

The main macros that get enlarged by this change are
    define OMPI_ARRAY_LOGICAL_2_INT_ALLOC : mallocs now
    define OMPI_ARRAY_LOGICAL_2_INT : also mallocs now
But these are only used in 4 places, three of which are the purpose of
this checkin, to avoid the former in-place expansion of an INPUT arg:
    cart_create_f.c
    cart_map_f.c
    cart_sub_f.c
and one of which is an OUPUT arg that was fine and that gets
unnecessarily expanded into a separate array by this checkin.
    cart_get_f.c

So I think an unnecessary malloc in cart_get_f.c is the only downside
to this change, where the logicals array argument could have been used
and converted in place.

Signed-off-by: Mark Allen <markalle@us.ibm.com>

Update provided by Gilles Gouaillardet to keep the in-place option
if OMPI_FORTRAN_VALUE_TRUE == 1 where no conversion is needed.

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
2019-04-01 10:38:05 -04:00

102 строки
3.9 KiB
C

/*
* Copyright (c) 2004-2005 The Trustees of Indiana University and Indiana
* University Research and Technology
* Corporation. All rights reserved.
* Copyright (c) 2004-2005 The University of Tennessee and The University
* of Tennessee Research Foundation. All rights
* reserved.
* Copyright (c) 2004-2007 High Performance Computing Center Stuttgart,
* University of Stuttgart. All rights reserved.
* Copyright (c) 2004-2005 The Regents of the University of California.
* All rights reserved.
* Copyright (c) 2011-2012 Cisco Systems, Inc. All rights reserved.
* Copyright (c) 2015 Research Organization for Information Science
* and Technology (RIST). All rights reserved.
* Copyright (c) 2019 IBM Corporation. All rights reserved.
* $COPYRIGHT$
*
* Additional copyrights may follow
*
* $HEADER$
*/
#include "ompi_config.h"
#include "ompi/mpi/fortran/mpif-h/bindings.h"
#if OMPI_BUILD_MPI_PROFILING
#if OPAL_HAVE_WEAK_SYMBOLS
#pragma weak PMPI_CART_CREATE = ompi_cart_create_f
#pragma weak pmpi_cart_create = ompi_cart_create_f
#pragma weak pmpi_cart_create_ = ompi_cart_create_f
#pragma weak pmpi_cart_create__ = ompi_cart_create_f
#pragma weak PMPI_Cart_create_f = ompi_cart_create_f
#pragma weak PMPI_Cart_create_f08 = ompi_cart_create_f
#else
OMPI_GENERATE_F77_BINDINGS (PMPI_CART_CREATE,
pmpi_cart_create,
pmpi_cart_create_,
pmpi_cart_create__,
pompi_cart_create_f,
(MPI_Fint *old_comm, MPI_Fint *ndims, MPI_Fint *dims, ompi_fortran_logical_t *periods, ompi_fortran_logical_t *reorder, MPI_Fint *comm_cart, MPI_Fint *ierr),
(old_comm, ndims, dims, periods, reorder, comm_cart, ierr) )
#endif
#endif
#if OPAL_HAVE_WEAK_SYMBOLS
#pragma weak MPI_CART_CREATE = ompi_cart_create_f
#pragma weak mpi_cart_create = ompi_cart_create_f
#pragma weak mpi_cart_create_ = ompi_cart_create_f
#pragma weak mpi_cart_create__ = ompi_cart_create_f
#pragma weak MPI_Cart_create_f = ompi_cart_create_f
#pragma weak MPI_Cart_create_f08 = ompi_cart_create_f
#else
#if ! OMPI_BUILD_MPI_PROFILING
OMPI_GENERATE_F77_BINDINGS (MPI_CART_CREATE,
mpi_cart_create,
mpi_cart_create_,
mpi_cart_create__,
ompi_cart_create_f,
(MPI_Fint *old_comm, MPI_Fint *ndims, MPI_Fint *dims, ompi_fortran_logical_t *periods, ompi_fortran_logical_t *reorder, MPI_Fint *comm_cart, MPI_Fint *ierr),
(old_comm, ndims, dims, periods, reorder, comm_cart, ierr) )
#else
#define ompi_cart_create_f pompi_cart_create_f
#endif
#endif
void ompi_cart_create_f(MPI_Fint *old_comm, MPI_Fint *ndims, MPI_Fint *dims,
ompi_fortran_logical_t *periods, ompi_fortran_logical_t *reorder,
MPI_Fint *comm_cart, MPI_Fint *ierr)
{
MPI_Comm c_comm1, c_comm2;
int size, c_ierr;
OMPI_ARRAY_NAME_DECL(dims);
OMPI_LOGICAL_ARRAY_NAME_DECL(periods);
c_comm1 = PMPI_Comm_f2c(*old_comm);
size = OMPI_FINT_2_INT(*ndims);
OMPI_ARRAY_FINT_2_INT(dims, size);
OMPI_ARRAY_LOGICAL_2_INT(periods, size);
c_ierr = PMPI_Cart_create(c_comm1, size,
OMPI_ARRAY_NAME_CONVERT(dims),
OMPI_LOGICAL_ARRAY_NAME_CONVERT(periods),
OMPI_LOGICAL_2_INT(*reorder),
&c_comm2);
if (NULL != ierr) *ierr = OMPI_INT_2_FINT(c_ierr);
if (MPI_SUCCESS == c_ierr) {
*comm_cart = PMPI_Comm_c2f(c_comm2);
}
/*
* Need to convert back into Fortran, to not surprise the user
*/
OMPI_ARRAY_FINT_2_INT_CLEANUP(dims);
OMPI_ARRAY_LOGICAL_2_INT_CLEANUP(periods);
}