[mpich-commits] [mpich] MPICH primary repository branch, master, updated. v3.0.4-110-g3d96864
mysql vizuser
noreply at mpich.org
Thu May 2 12:02:39 CDT 2013
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "MPICH primary repository".
The branch, master has been updated
via 3d96864e665e281a543383ec7e68b25d1861276a (commit)
from d9a67f406163643fb86ab2d10e2853f20ad11a1d (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
http://git.mpich.org/mpich.git/commitdiff/3d96864e665e281a543383ec7e68b25d1861276a
commit 3d96864e665e281a543383ec7e68b25d1861276a
Author: William Gropp <wgropp at illinois.edu>
Date: Thu May 2 12:49:26 2013 -0400
Fixes for #1804 and #1828, related to alignment issues with MPI_Status
There are two alignment-related problems with MPI_STATUS arguments in
Fortran. The first is that while MPI_STATUS is an INTEGER array in
Fortran, in C there is an element of type MPI_Count. MPI_Count may
be longer than an INTEGER and thus a C MPI_Status may have a stricter
alignment requirement than a Fortran INTEGER array. The fixes to
src/binding/f77/buildiface ensures that STATUS arguments that are
provided to C by the Fortran wrappers are properly aligned, while avoiding
copies when possible. The second fix deals with padding that the
introduction of MPI_Count as the count type into the C structure for
status will often introduce.
diff --git a/.gitignore b/.gitignore
index 1634a5a..ed5203c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -159,6 +159,7 @@ Makefile.am-stamp
/README.envvar
/maint/extracterrmsgs
/src/binding/f90/mpi_base.f90.in
+/src/binding/f90/mpi_constants.f90.in
/src/env/mpixxx_opts.conf
/src/mpi/romio/include/mpio.h
/src/mpi/romio/include/mpiof.h
diff --git a/configure.ac b/configure.ac
index 6b7237c..f3d3c2d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -4079,9 +4079,10 @@ dnl len_doublecplx=$hexlen
#
PAC_PROG_FC_INT_KIND(INTEGER_KIND,$pac_cv_f77_sizeof_integer,$CROSS_F90_INTEGER_KIND)
if test "$INTEGER_KIND" = "-1" ; then
- # Wild guess; probably means that Fortran 90 is not available
- AC_MSG_WARN([Unable to determine Fortran 90 KIND values for either address-sized integers or offset-sized integers. Using 4 in that case.])
- INTEGER_KIND=4
+ # In our experience, this usually means that there is some
+ # problem building and/or running the f90 program. Fail
+ # in this case rather than attempt to continue
+ AC_MSG_ERROR([Unable to determine Fortran 90 KIND values for either address-sized integers or offset-sized integers.])
fi
#
# Some compilers won't allow a -1 kind (e.g., absoft). In this case,
@@ -5792,6 +5793,8 @@ if test "$enable_f77" = yes -a -z "$MPI_STATUS_SIZE" ; then
if test $MPI_SIZEOF_COUNT -gt $pac_cv_f77_sizeof_integer ; then
# Alignment test looks at the low address bits, as long
# as the sizeof(MPI_Count) is 2^j.
+ # If we can't tell, pick a value that forces the Fortran
+ # wrappers to use a local copy.
MPIR_COUNT_ALIGNMENT=0
case $MPI_SIZEOF_COUNT in
4) MPIR_COUNT_ALIGNMENT="0x3";;
@@ -5801,6 +5804,77 @@ if test "$enable_f77" = yes -a -z "$MPI_STATUS_SIZE" ; then
esac
AC_DEFINE_UNQUOTED([MPIR_COUNT_ALIGNMENT],[$MPIR_COUNT_ALIGNMENT],[Value that anded with an address will give 0 if the address is properly aligned for MPI_Count])
+
+ # Compare the size of MPI_Status to the sizes of the individual
+ # elements. If the sizeof(MPI_Status) > sum of the sizes, then
+ # determine if simple padding will work.
+ # FIXME: DETERMINED TO SEE IF THERE IS PROBLEM BUT NOT YET USED
+ AC_MSG_CHECKING([whether MPI_Status contains padding])
+ sum_of_status_element_sizes=`expr 6 \* $ac_cv_sizeof_int + $MPI_SIZEOF_COUNT`
+ if test $sum_of_status_element_sizes -lt $pac_cv_sizeof_mpi_status ; then
+ AC_MSG_RESULT([yes])
+ if test "$enable_fc" = "yes" ; then
+ # FIXME: We could compute the padding or delete the
+ # status definition from mpi_constants
+ AC_MSG_CHECKING([padding size before count field in MPI_Status])
+ rm -f pac_mpi_status.h
+dnl quote with [] since otherwise m4 strips the array declaration out of this
+dnl structure
+[ cat > pac_mpi_status.h <<_EOF
+typedef struct {
+ int MPI_SOURCE;
+ int MPI_TAG;
+ int MPI_ERROR;
+ $MPI_COUNT count;
+ int cancelled;
+ int abi_slush_fund[2];
+ $EXTRA_STATUS_DECL
+} MPI_Status;
+MPI_Status mya;
+_EOF
+]
+ AC_COMPUTE_INT(pac_cv_fc_pad1,[((char*)&mya.count-(((char*)&mya)+3*sizeof(int)))],[AC_INCLUDES_DEFAULT()
+#include "pac_mpi_status.h"])
+ AC_MSG_RESULT($pac_cv_fc_pad1)
+ AC_MSG_CHECKING([padding at end of MPI_Status])
+ AC_COMPUTE_INT(pac_cv_fc_pad2,[((char*)(&mya + 1) - ((char *)(mya.abi_slush_fund+1) + sizeof(int)))],[AC_INCLUDES_DEFAULT()
+#include "pac_mpi_status.h"])
+ AC_MSG_RESULT($pac_cv_fc_pad2)
+ rm -f pac_mpi_status.h
+ pac_cv_pad1found=no
+ pac_cv_pad2found=no
+ FC_STATUS_PAD1=""
+ FC_STATUS_PAD2=""
+ AC_SUBST(FC_STATUS_PAD1)
+ AC_SUBST(FC_STATUS_PAD2)
+ if test $pac_cv_fc_pad1 -gt 0 ; then
+ if test $pac_cv_fc_pad1 -eq $ac_cv_sizeof_int ; then
+ pac_cv_pad1found=yes
+ FC_STATUS_PAD1="INTEGER :: PAD1"
+ fi
+ else
+ # No padding needed here, so successfully determined
+ # padding
+ pac_cv_pad1found=yes
+ fi
+ if test $pac_cv_fc_pad2 -gt 0 ; then
+ if test $pac_cv_fc_pad2 -eq $ac_cv_sizeof_int ; then
+ pac_cv_pad2found=yes
+ FC_STATUS_PAD2="INTEGER :: PAD2"
+ fi
+ else
+ # No padding needed here, so successfully determined
+ # padding
+ pac_cv_pad2found=yes
+ fi
+ if test $pac_cv_pad1found = "no" -o \
+ $pac_cv_pad2found = "no" ; then
+ AC_MSG_ERROR([INTERNAL ERROR: definition of MPI_Status in C and Fortran 90 is incompatible due to padding in the C definition])
+ fi
+ fi
+ else
+ AC_MSG_RESULT([no])
+ fi
fi
fi
AC_SUBST([MPI_STATUS_SIZE])
@@ -6063,6 +6137,7 @@ AC_OUTPUT(Makefile \
src/binding/f77/setbot.c \
src/binding/f90/mpi_sizeofs.f90 \
src/binding/f90/mpi_base.f90 \
+ src/binding/f90/mpi_constants.f90 \
src/packaging/pkgconfig/mpich.pc \
src/packaging/envmods/mpich.module \
src/env/mpixxx_opts.conf \
diff --git a/src/binding/f77/buildiface b/src/binding/f77/buildiface
index f6c6611..5f30c6d 100755
--- a/src/binding/f77/buildiface
+++ b/src/binding/f77/buildiface
@@ -2553,10 +2553,18 @@ sub status_out_ftoc {
# casting to a pointer with greater alignment (and which the
# specific runtime check is used to ensure the alignments are
# ok).
+ # Horrible requirement: MPI Standard (see MPI-3, Section 3.2.5,
+ # page 30, lines 39-43) implies that the MPI_ERROR field needs
+ # to be *preserved*. Thus, if we have a temp status, we need to
+ # either copy it to the temp status (as here) or *not* copy over
+ # it when storing the result. This is simpler.
print $OUTFD "\
- if (l$count != MPI_STATUS_IGNORE &&
- (((MPI_Aint)v$count) & MPIR_COUNT_ALIGNMENT) == 0)
- l$count = (MPI_Status*)(void *)v$count;\n";
+ if (l$count != MPI_STATUS_IGNORE) {
+ if ((((MPI_Aint)v$count) & MPIR_COUNT_ALIGNMENT) == 0)
+ l$count = (MPI_Status*)(void *)v$count;
+ else
+ l$count->MPI_ERROR = ((MPI_Status*)v$count)->MPI_ERROR;
+ }\n";
}
}
sub status_out_arg {
@@ -2679,9 +2687,17 @@ sub status_array_out_fnulltoc {
}
sub status_array_out_ftoc {
my $count = $_[0];
-
+
+ # See discussion of status_out_ftoc - unfortunately, we need to
+ # copy *just* the MPI_ERROR field
if ($within_fint) {
- print $OUTFD " if (l$count != MPI_STATUSES_IGNORE) { l$count = (MPI_Status*)$malloc($Array_size * sizeof(MPI_Status)); }\n";
+ print $OUTFD "\
+ if (l$count != MPI_STATUSES_IGNORE) {
+ int li;
+ l$count = (MPI_Status*)$malloc($Array_size * sizeof(MPI_Status));
+ for (li=0; li<$Array_size; li++)
+ l$count\[li].MPI_ERROR = ((MPI_Status*)v$count)[li].MPI_ERROR;
+ }\n";
}
elsif ($fixStatusAlignment) {
print $OUTFD "\
@@ -2689,8 +2705,11 @@ sub status_array_out_ftoc {
if( (((MPI_Aint)v$count) & MPIR_COUNT_ALIGNMENT) == 0)
l$count = (MPI_Status*)(void *)v$count;
else {
+ int li;
lalloc$count = 1;
l$count = (MPI_Status*)$malloc($Array_size * sizeof(MPI_Status));
+ for (li=0; li<$Array_size; li++)
+ l$count\[li].MPI_ERROR = ((MPI_Status*)v$count)[li].MPI_ERROR;
}
}\n";
}
@@ -2714,7 +2733,7 @@ sub status_array_ctof {
" if ($testFlag$coutvar != MPI_STATUSES_IGNORE) {
int li;
for (li=0; li<$ActSize; li++) {
- MPI_Status_c2f($coutvar+li,$outvar+li*5);
+ MPI_Status_c2f($coutvar+li,$outvar+li*MPIF_STATUS_SIZE);
}
}\n";
$clean_up .= " if ($coutvar != MPI_STATUSES_IGNORE) { $free($coutvar); }\n";
@@ -2732,7 +2751,7 @@ sub status_array_ctof {
" if (${testFlag}lalloc$count) {
int li;
for (li=0; li<$ActSize; li++) {
- MPI_Status_c2f($coutvar+li,$outvar+li*5);
+ MPI_Status_c2f($coutvar+li,$outvar+li*MPIF_STATUS_SIZE);
}
}\n";
$clean_up .= " if (lalloc$count) { $free($coutvar); }\n";
diff --git a/src/binding/f90/buildiface b/src/binding/f90/buildiface
index c5eb901..d84ef1f 100755
--- a/src/binding/f90/buildiface
+++ b/src/binding/f90/buildiface
@@ -603,7 +603,7 @@ print MPIBASEFD " END MODULE ${ucoutfile_prefix}_BASE\n";
close MPIBASEFD;
&ReplaceIfDifferent( "${outfile_prefix}_base.f90.in", "${outfile_prefix}_base.f90.in.new" );
-open ( MPIFD, ">${outfile_prefix}_constants.f90.new" ) || die "Cannot open ${outfile_prefix}_constants.f90.new\n";
+open ( MPIFD, ">${outfile_prefix}_constants.f90.in.new" ) || die "Cannot open ${outfile_prefix}_constants.f90.in.new\n";
print MPIFD "! -*- Mode: Fortran; -*-
! (C) 2008 by Argonne National Laboratory.
! See COPYRIGHT in top-level directory.
@@ -642,13 +642,21 @@ $PRIVATEVAR = "";
#
#$BINDACCESS = ", BIND(C)";
#$BINDDEF ="";
+# Yet another problem.
+# Because MPI_Count may be longer than a single (Fortran) INTEGER,
+# alignment restrictions may introduce padding in the structure
+# And one more problem: If a Fortran INTEGER is not the same as a C int,
+# then these are also wrong (see the "fint" option in
+# src/binding/f77/buildiface
print MPIFD <<EOT;
TYPE$BINDACCESS :: MPI_Status
$BINDDEF
INTEGER$PUBLICVAR :: MPI_SOURCE, MPI_TAG, MPI_ERROR
+ \@FC_STATUS_PAD1\@
INTEGER$PRIVATEVAR(KIND=MPI_COUNT_KIND) :: count
INTEGER$PRIVATEVAR :: cancelled
INTEGER$PRIVATEVAR(2) :: abi_slush_fund
+ \@FC_STATUS_PAD2\@
END TYPE MPI_Status
EOT
@@ -708,7 +716,7 @@ foreach $handle (keys(%handles)) {
print MPIFD " END MODULE ${ucoutfile_prefix}_CONSTANTS\n";
close MPIFD;
-&ReplaceIfDifferent( "${outfile_prefix}_constants.f90", "${outfile_prefix}_constants.f90.new" );
+&ReplaceIfDifferent( "${outfile_prefix}_constants.f90.in", "${outfile_prefix}_constants.f90.in.new" );
#
# Generate the choice argument routines
-----------------------------------------------------------------------
Summary of changes:
.gitignore | 1 +
configure.ac | 81 ++++++++++++++++++++++++++++++++++++++++++--
src/binding/f77/buildiface | 33 ++++++++++++++----
src/binding/f90/buildiface | 12 +++++-
4 files changed, 115 insertions(+), 12 deletions(-)
hooks/post-receive
--
MPICH primary repository
More information about the commits
mailing list