[mpich-commits] r10610 - in mpich2/trunk: src/mpid/common/datatype/dataloop test/mpi/datatype

14 Nov 2012

Author: goodell
Date: 2012-11-14 11:20:14 -0600 (Wed, 14 Nov 2012)
New Revision: 10610

Modified:
   mpich2/trunk/src/mpid/common/datatype/dataloop/dataloop_create_indexed.c
   mpich2/trunk/test/mpi/datatype/indexed-misc.c
Log:
fix hindexed pointer-casting bugs

Bug report from Dmitry Durnov @ Intel.  This patch also includes a fix
based on code suggested by Dmitry.

No reviewer.

Modified: mpich2/trunk/src/mpid/common/datatype/dataloop/dataloop_create_indexed.c
===================================================================

--- mpich2/trunk/src/mpid/common/datatype/dataloop/dataloop_create_indexed.c	2012-11-12 17:07:38 UTC (rev 10609)
+++ mpich2/trunk/src/mpid/common/datatype/dataloop/dataloop_create_indexed.c	2012-11-14 17:20:14 UTC (rev 10610)
@@ -137,9 +137,14 @@
      */
     if (contig_count == 1)
     {
+        const void *disp_arr_tmp; /* no ternary assignment to avoid clang warnings */
+        if (dispinbytes)
+            disp_arr_tmp = &(((const MPI_Aint *)displacement_array)[first]);
+        else
+            disp_arr_tmp = &(((const int *)displacement_array)[first]);
 	err = PREPEND_PREFIX(Dataloop_create_blockindexed)(1,
 							   (int) old_type_count,
-							   &(((int *)displacement_array)[first]),
+							   disp_arr_tmp,
 							   dispinbytes,
 							   oldtype,
 							   dlp_p,
@@ -166,9 +171,14 @@
     }
     if (blksz == blocklength_array[first])
     {
+        const void *disp_arr_tmp; /* no ternary assignment to avoid clang warnings */
+        if (dispinbytes)
+            disp_arr_tmp = &(((const MPI_Aint *)displacement_array)[first]);
+        else
+            disp_arr_tmp = &(((const int *)displacement_array)[first]);
 	err = PREPEND_PREFIX(Dataloop_create_blockindexed)(icount-first,
 							   blksz,
-							   &(((int *)displacement_array)[first]),
+							   disp_arr_tmp,
 							   dispinbytes,
 							   oldtype,
 							   dlp_p,

Modified: mpich2/trunk/test/mpi/datatype/indexed-misc.c
===================================================================
--- mpich2/trunk/test/mpi/datatype/indexed-misc.c	2012-11-12 17:07:38 UTC (rev 10609)
+++ mpich2/trunk/test/mpi/datatype/indexed-misc.c	2012-11-14 17:20:14 UTC (rev 10610)
@@ -10,14 +10,38 @@
 #ifdef HAVE_STRING_H
 #include <string.h>
 #endif
+#include <assert.h>
+#include <limits.h>
 
-static int verbose = 0;
+static int verbose = 1;
 
+#define check(cond_)                                                                             \
+    do {                                                                                         \
+        if (!(cond_)) {                                                                          \
+            if (verbose) {                                                                       \
+                fprintf(stderr, "condition '%s' does not hold, at line %d\n", #cond_, __LINE__); \
+            }                                                                                    \
+            errs += 1;                                                                           \
+        }                                                                                        \
+    } while (0)
+
+#define check_err(err_, what_failed_)                                                 \
+    do {                                                                              \
+        if (err_) {                                                                   \
+            if (verbose) {                                                            \
+                fprintf(stderr, "error: %s, at line %d\n", (what_failed_), __LINE__); \
+            }                                                                         \
+            errs += (err_);                                                           \
+        }                                                                             \
+    } while (0)
+
 /* tests */
 int indexed_contig_test(void);
 int indexed_zeroblock_first_test(void);
 int indexed_zeroblock_middle_test(void);
 int indexed_zeroblock_last_test(void);
+int indexed_contig_leading_zero_test(void);
+int indexed_same_lengths(void);
 
 /* helper functions */
 int parse_args(int argc, char **argv);
@@ -62,6 +86,18 @@
 				err);
     errs += err;
 
+    err = indexed_contig_leading_zero_test();
+    if (err && verbose) fprintf(stderr,
+                                "%d errors in indexed_contig_leading_zero_test.\n",
+                                err);
+    errs += err;
+
+    err = indexed_same_lengths();
+    if (err && verbose) fprintf(stderr,
+                                "%d errors in indexed_contig_leading_zero_test.\n",
+                                err);
+    errs += err;
+
     /* print message and exit */
     if (errs) {
 	fprintf(stderr, "Found %d errors\n", errs);
@@ -299,6 +335,280 @@
     return errs;
 }
 
+/* very similar to indexed_zeroblock_first_test, but only has a single contig in
+ * order to catch a particular optimization path in MPICH's
+ * Dataloop_create_indexed routine */
+int indexed_contig_leading_zero_test(void)
+{
+    int err, errs = 0;
+
+    int i;
+    MPI_Datatype type = MPI_DATATYPE_NULL;
+    MPI_Datatype struct_type = MPI_DATATYPE_NULL;
+    MPI_Datatype types[2];
+    int len[3]  = { 0, 4, 0 };
+    int disp[3] = { INT_MAX, 2, INT_MAX};
+    MPI_Aint adisp[3];
+    MPI_Aint lb, ub;
+    int *buf = NULL;
+
+    err = MPI_Type_indexed(3, len, disp, MPI_INT, &type);
+    check_err(err, "creating indexed type in indexed_contig_leading_zero_test()");
+    err = MPI_Type_commit(&type);
+    check_err(err, "committing indexed type in indexed_contig_leading_zero_test()");
+
+    MPI_Type_lb(type, &lb);
+    check(lb == 2 * sizeof(int));
+    MPI_Type_ub(type, &ub);
+    check(ub == 6 * sizeof(int));
+
+    /* make sure packing/unpacking works (hits a simple "is_contig" case in
+     * MPICH's pack/unpack routines) */
+    buf = malloc(10*sizeof(int));
+    assert(buf != NULL);
+    for (i = 0; i < 10; ++i) {
+        buf[i] = i + 1;
+    }
+    err = pack_and_unpack((char *) buf, 1, type, 10 * sizeof(int));
+    check_err(err, "packing/unpacking in indexed_contig_leading_zero_test()");
+    for (i = 0; i < 10; ++i) {
+        int expected;
+        if (i >= 2 && i < 6)
+            expected = i + 1;
+        else
+            expected = 0;
+        check(buf[i] == expected);
+    }
+    free(buf);
+
+    /* -------------------------------------------------------------------- */
+    /* A more rigorous test of the indexed type.  Use a hard-to-optimize struct
+     * type to force a more complicated datatype processing path
+     * (MPID_Segment_manipulate in MPICH) */
+    len[0] = 1;
+    len[1] = 1;
+    adisp[0] = 0;
+    adisp[1] = 8*sizeof(int);
+    types[0] = type;
+    types[1] = MPI_INT;
+
+    /* struct layout: xx0123xx4x ('x' indicates a hole), one char is an
+     * MPI_INT */
+    MPI_Type_create_struct(2, len, adisp, types, &struct_type);
+    check_err(err, "creating struct type in indexed_contig_leading_zero_test()");
+    err = MPI_Type_commit(&struct_type);
+    check_err(err, "committing struct type in indexed_contig_leading_zero_test()");
+
+    buf = malloc(10*sizeof(int));
+    assert(buf != NULL);
+    for (i = 0; i < 10; ++i) {
+        buf[i] = i + 1;
+    }
+    err = pack_and_unpack((char *) buf, 1, struct_type, 10 * sizeof(int));
+    check_err(err, "packing/unpacking in indexed_contig_test()");
+
+    for (i = 0; i < 10; ++i) {
+        int expected;
+        if ((i >= 2 && i < 6) || i == 8)
+            expected = i + 1;
+        else
+            expected = 0;
+        check(buf[i] == expected);
+    }
+    free(buf);
+
+    MPI_Type_free(&struct_type);
+    MPI_Type_free( &type );
+
+    /* -------------------------------------------------------------------- */
+    /* now do the same as above, but with hindexed */
+    len[0] = 0;
+    len[1] = 4;
+    len[2] = 0;
+    /* use *_MAX vars to improve our chances of hitting any pointer-casting
+     * bugs in a big way (segfaults, etc.) */
+    if (sizeof(MPI_Aint) == sizeof(long long)) {
+        adisp[0] = (MPI_Aint)LLONG_MAX;
+        adisp[1] = 2*sizeof(int);
+        adisp[2] = (MPI_Aint)LLONG_MAX;
+    }
+    else {
+        adisp[0] = (MPI_Aint)INT_MAX;
+        adisp[1] = 2*sizeof(int);
+        adisp[2] = (MPI_Aint)INT_MAX;
+    }
+
+    err = MPI_Type_hindexed(3, len, adisp, MPI_INT, &type);
+    check_err(err, "creating hindexed type in indexed_contig_leading_zero_test()");
+
+    err = MPI_Type_commit(&type);
+    check_err(err, "committing hindexed type in indexed_contig_leading_zero_test()");
+
+    MPI_Type_lb(type, &lb);
+    check(lb == 2 * sizeof(int));
+    MPI_Type_ub(type, &ub);
+    check(ub == 6 * sizeof(int));
+
+    buf = malloc(10*sizeof(int));
+    assert(buf != NULL);
+    for (i = 0; i < 10; ++i) {
+        buf[i] = i + 1;
+    }
+    err = pack_and_unpack((char *) buf, 1, type, 10 * sizeof(int));
+    check_err(err, "packing/unpacking in indexed_contig_test()");
+
+    for (i = 0; i < 10; ++i) {
+        int expected;
+        if (i >= 2 && i < 6)
+            expected = i + 1;
+        else
+            expected = 0;
+        check(buf[i] == expected);
+    }
+    free(buf);
+
+    /* -------------------------------------------------------------------- */
+    /* A more rigorous test of the hindexed type.  Use a hard-to-optimize struct
+     * type to force a more complicated datatype processing path
+     * (MPID_Segment_manipulate in MPICH) */
+    len[0] = 1;
+    len[1] = 1;
+    adisp[0] = 0;
+    adisp[1] = 8*sizeof(int);
+
+    /* struct layout: xx0123xx4x ('x' indicates a hole), one char is an
+     * MPI_INT */
+    err = MPI_Type_create_struct(2, len, adisp, types, &struct_type);
+    check_err(err, "committing struct type in indexed_contig_leading_zero_test()");
+    err = MPI_Type_commit(&struct_type);
+    check_err(err, "committing struct type in indexed_contig_leading_zero_test()");
+
+    buf = malloc(10*sizeof(int));
+    assert(buf != NULL);
+    for (i = 0; i < 10; ++i) {
+        buf[i] = i + 1;
+    }
+    /* fails in old MPICH (3.0rc1 and earlier), despite correct ub/lb
+     * determination */
+    err = pack_and_unpack((char *) buf, 1, struct_type, 10 * sizeof(int));
+    check_err(err, "packing/unpacking in indexed_contig_test()");
+
+    for (i = 0; i < 10; ++i) {
+        int expected;
+        if ((i >= 2 && i < 6) || i == 8)
+            expected = i + 1;
+        else
+            expected = 0;
+        check(buf[i] == expected);
+    }
+    free(buf);
+
+    MPI_Type_free(&struct_type);
+    MPI_Type_free(&type);
+
+    return errs;
+}
+
+/* Test an indexed (and hindexed) type where the block length is the same for
+ * all blocks, but with differing displacements so that it cannot directly be
+ * converted to a vector type.  It is also important to add a dummy element at
+ * the beginning in order to cause int/MPI_Aint misalignment for the
+ * displacement of the first non-zero-width component. */
+int indexed_same_lengths(void)
+{
+    int err, errs = 0;
+
+    int i;
+    MPI_Datatype type = MPI_DATATYPE_NULL;
+    int len[4];
+    int disp[4];
+    MPI_Aint adisp[4];
+    MPI_Aint lb, ub;
+    int *buf = NULL;
+
+    len[0] = 0;
+    len[1] = 1;
+    len[2] = 1;
+    len[3] = 1;
+
+    disp[0] = 0;
+    disp[1] = 1;
+    disp[2] = 3;
+    disp[3] = 8;
+
+    err = MPI_Type_indexed(4, len, disp, MPI_INT, &type);
+    check_err(err, "creating indexed type in indexed_same_lengths()");
+    err = MPI_Type_commit(&type);
+    check_err(err, "committing indexed type in indexed_same_lengths()");
+
+    MPI_Type_lb(type, &lb);
+    check(lb == 1 * sizeof(int));
+    MPI_Type_ub(type, &ub);
+    check(ub == 9 * sizeof(int));
+
+    buf = malloc(10*sizeof(int));
+    assert(buf != NULL);
+    for (i = 0; i < 10; ++i) {
+        buf[i] = i + 1;
+    }
+    err = pack_and_unpack((char *) buf, 1, type, 10 * sizeof(int));
+    check_err(err, "packing/unpacking in indexed_same_lengths()");
+    for (i = 0; i < 10; ++i) {
+        int expected;
+        if (i == 1 || i == 3 || i == 8)
+            expected = i + 1;
+        else
+            expected = 0;
+        check(buf[i] == expected);
+    }
+    free(buf);
+
+    MPI_Type_free(&type);
+
+    /* -------------------------------------------------------------------- */
+    /* now do the same as above, but with hindexed */
+    len[0] = 0;
+    len[1] = 1;
+    len[2] = 1;
+    len[3] = 1;
+
+    adisp[0] = 0 * sizeof(int);
+    adisp[1] = 1 * sizeof(int);
+    adisp[2] = 3 * sizeof(int);
+    adisp[3] = 8 * sizeof(int);
+
+    err = MPI_Type_hindexed(4, len, adisp, MPI_INT, &type);
+    check_err(err, "creating hindexed type in indexed_same_lengths()");
+    err = MPI_Type_commit(&type);
+    check_err(err, "committing hindexed type in indexed_same_lengths()");
+
+    MPI_Type_lb(type, &lb);
+    check(lb == 1 * sizeof(int));
+    MPI_Type_ub(type, &ub);
+    check(ub == 9 * sizeof(int));
+
+    buf = malloc(10*sizeof(int));
+    assert(buf != NULL);
+    for (i = 0; i < 10; ++i) {
+        buf[i] = i + 1;
+    }
+    err = pack_and_unpack((char *) buf, 1, type, 10 * sizeof(int));
+    check_err(err, "packing/unpacking in indexed_same_lengths()");
+    for (i = 0; i < 10; ++i) {
+        int expected;
+        if (i == 1 || i == 3 || i == 8)
+            expected = i + 1;
+        else
+            expected = 0;
+        check(buf[i] == expected);
+    }
+    free(buf);
+
+    MPI_Type_free(&type);
+
+    return errs;
+}
+
 /* pack_and_unpack()
  *
  * Perform packing and unpacking of a buffer for the purposes of checking

    

[mpich-commits] r10610 - in mpich2/trunk: src/mpid/common/datatype/dataloop test/mpi/datatype

goodell＠mcs.anl.gov