ompi/datatype: define OMPI_DATATYPE_INIT_UNAVAILABLE_BASIC_TYPE macro

[ggouaillardet]

and properly handle the case when ptrdiff_t is not found

Thanks Ilias Miroslav for the report

[ggouaillardet]

@bosilca can you please have a look at this ?
the issue was initially reported at https://www.open-mpi.org/community/lists/users/2016/05/29142.php
and though i believe this is caused by an user mistake (use PGI preprocessor with GNU compilers) the
OMPI_DATATYPE_INIT_UNAVAILABLE_BASIC_TYPE macro is not defined, and SIZEOF_PTRDIFF_T is used even if the macro is not defined (because ptrdiff_t is not available)

first, the compilation will alway fail if heterogeneous support is enabled

#if OPAL_ENABLE_HETEROGENEOUS_SUPPORT
[...]
#if defined(HAVE_PTRDIFF_T) && SIZEOF_PTRDIFF_T == 4
            array_of_disp[i] = opal_swap_bytes4(array_of_disp[i]);
#elif defined(HAVE_PTRDIFF_T) && SIZEOF_PTRDIFF_T == 8
            array_of_disp[i] = (MPI_Aint)opal_swap_bytes8(array_of_disp[i]);
#else
#error "Unknown size of ptrdiff_t"
#endif

then, even if HAVE_PTRDIFF_T is undefined (and hence SIZEOF_PTRDIFF_T is undefined too)
OPAL_PTRDIFF_T is defined.
should we use the SIZEOF_OPAL_PTRDIFF_T macro (that has to be defined) instead ?

also, ptrdiff_t is used in several places in ompi/mca/coll and ompi/mca/osc
should we use OPAL_PTRDIFF_T instead ?

[bosilca]

I think the correct approach will be to use OPAL_PTRDIFF everywhere where we need a correspondence between our internal type and the MPI view of MPI_AINT (and MPI_COUNT).

diff --git a/configure.ac b/configure.ac
index b965d55..b702455 100644
--- a/configure.ac
+++ b/configure.ac
@@ -737,6 +737,8 @@ else
 fi
 AC_DEFINE_UNQUOTED([OPAL_PTRDIFF_TYPE], [$opal_ptrdiff_t],
     [type to use for ptrdiff_t])
+AC_DEFINE_UNQUOTED([SIZEOF_OPAL_PTRDIFF_TYPE], [$opal_ptrdiff_size],
+    [size of the type in use for ptrdiff_t])
 AC_MSG_RESULT([$opal_ptrdiff_t (size: $opal_ptrdiff_size)])

 #
diff --git a/ompi/datatype/ompi_datatype_args.c b/ompi/datatype/ompi_datatype_args.c
index 5a62ac5..a16011c 100644
--- a/ompi/datatype/ompi_datatype_args.c
+++ b/ompi/datatype/ompi_datatype_args.c
@@ -3,7 +3,7 @@
  * Copyright (c) 2004-2007 The Trustees of Indiana University and Indiana
  *                         University Research and Technology
  *                         Corporation.  All rights reserved.
- * Copyright (c) 2004-2013 The University of Tennessee and The University
+ * Copyright (c) 2004-2016 The University of Tennessee and The University
  *                         of Tennessee Research Foundation.  All rights
  *                         reserved.
  * Copyright (c) 2004-2006 High Performance Computing Center Stuttgart,
@@ -647,9 +649,9 @@ static ompi_datatype_t* __ompi_datatype_create_from_packed_description( void** p
             array_of_length[i] = opal_swap_bytes4(array_of_length[i]);
         }
         for (i = 0 ; i < number_of_disp ; ++i) {
-#if SIZEOF_PTRDIFF_T == 4
+#if SIZEOF_OPAL_PTRDIFF_TYPE == 4
             array_of_disp[i] = opal_swap_bytes4(array_of_disp[i]);
-#elif SIZEOF_PTRDIFF_T == 8
+#elif SIZEOF_OPAL_PTRDIFF_TYPE == 8
             array_of_disp[i] = (MPI_Aint)opal_swap_bytes8(array_of_disp[i]);
 #else
 #error "Unknown size of ptrdiff_t"
diff --git a/ompi/datatype/ompi_datatype_module.c b/ompi/datatype/ompi_datatype_module.c
index e9d8521..4470cc2 100644
--- a/ompi/datatype/ompi_datatype_module.c
+++ b/ompi/datatype/ompi_datatype_module.c
@@ -3,7 +3,7 @@
  * Copyright (c) 2004-2006 The Trustees of Indiana University and Indiana
  *                         University Research and Technology
  *                         Corporation.  All rights reserved.
- * Copyright (c) 2004-2013 The University of Tennessee and The University
+ * Copyright (c) 2004-2016 The University of Tennessee and The University
  *                         of Tennessee Research Foundation.  All rights
  *                         reserved.
  * Copyright (c) 2004-2006 High Performance Computing Center Stuttgart,
@@ -273,13 +273,13 @@ ompi_predefined_datatype_t ompi_mpi_uint32_t = OMPI_DATATYPE_INIT_PREDEFINED(UIN
 ompi_predefined_datatype_t ompi_mpi_int64_t  = OMPI_DATATYPE_INIT_PREDEFINED( INT64_T, OMPI_DATATYPE_FLAG_DATA_C | OMPI_DATATYPE_FLAG_DATA_INT);
 ompi_predefined_datatype_t ompi_mpi_uint64_t = OMPI_DATATYPE_INIT_PREDEFINED(UINT64_T, OMPI_DATATYPE_FLAG_DATA_C | OMPI_DATATYPE_FLAG_DATA_INT);

-#if SIZEOF_PTRDIFF_T == 4
+#if SIZEOF_OPAL_PTRDIFF_TYPE == 4
 ompi_predefined_datatype_t ompi_mpi_aint = OMPI_DATATYPE_INIT_PREDEFINED_BASIC_TYPE(INT32_T, AINT, OMPI_DATATYPE_FLAG_DATA_C | OMPI_DATATYPE_FLAG_DATA_INT);
-#elif SIZEOF_PTRDIFF_T == 8
+#elif SIZEOF_OPAL_PTRDIFF_TYPE == 8
 ompi_predefined_datatype_t ompi_mpi_aint = OMPI_DATATYPE_INIT_PREDEFINED_BASIC_TYPE(INT64_T, AINT, OMPI_DATATYPE_FLAG_DATA_C | OMPI_DATATYPE_FLAG_DATA_INT);
 #else
 ompi_predefined_datatype_t ompi_mpi_aint = OMPI_DATATYPE_INIT_UNAVAILABLE_BASIC_TYPE(INT64_T, AINT, OMPI_DATATYPE_FLAG_DATA_C | OMPI_DATATYPE_FLAG_DATA_INT);
-#endif  /* SIZEOF_PTRDIFF_T == SIZEOF_LONG */
+#endif  /* SIZEOF_OPAL_PTRDIFF_TYPE == SIZEOF_LONG */

 #if OMPI_MPI_OFFSET_SIZE == 4
 ompi_predefined_datatype_t ompi_mpi_offset = OMPI_DATATYPE_INIT_PREDEFINED_BASIC_TYPE(UINT32_T, OFFSET, OMPI_DATATYPE_FLAG_DATA_C | OMPI_DATATYPE_FLAG_DATA_INT);
diff --git a/ompi/op/op.c b/ompi/op/op.c
index 31cd01f..8c044c8 100644
--- a/ompi/op/op.c
+++ b/ompi/op/op.c
@@ -3,7 +3,7 @@
  * Copyright (c) 2004-2006 The Trustees of Indiana University and Indiana
  *                         University Research and Technology
  *                         Corporation.  All rights reserved.
- * Copyright (c) 2004-2010 The University of Tennessee and The University
+ * Copyright (c) 2004-2016 The University of Tennessee and The University
  *                         of Tennessee Research Foundation.  All rights
  *                         reserved.
  * Copyright (c) 2004-2007 High Performance Computing Center Stuttgart,
@@ -187,9 +187,9 @@ int ompi_op_init(void)
     ompi_op_ddt_map[OMPI_DATATYPE_MPI_LONG_INT] = OMPI_OP_BASE_TYPE_LONG_INT;
     ompi_op_ddt_map[OMPI_DATATYPE_MPI_SHORT_INT] = OMPI_OP_BASE_TYPE_SHORT_INT;

-#if SIZEOF_PTRDIFF_T == 4
+#if SIZEOF_OPAL_PTRDIFF_TYPE == 4
     ompi_op_ddt_map[OMPI_DATATYPE_MPI_AINT] = OMPI_OP_BASE_TYPE_INT32_T;
-#elif SIZEOF_PTRDIFF_T == 8
+#elif SIZEOF_OPAL_PTRDIFF_TYPE == 8
     ompi_op_ddt_map[OMPI_DATATYPE_MPI_AINT] = OMPI_OP_BASE_TYPE_INT64_T;
 #else
 #warning Unsupported definition for MPI_AINT

[ggouaillardet]

:bot:retest

[ggouaillardet]

@bosilca i updated the PR based on your review, and also replaced ptrdiff_t with OPAL_PTRDIFF_TYPE in {opal,orte,ompi} projects.
oshmem uses ptrdiff_t in its public API, so i did not make any changes in this project.

[bosilca]

Thanks @ggouaillardet. Now that I see the entire change, I got concerned. Basically this change means that OMPI developers should stop using ptrdiff_t and instead always use OPAL_PTRDIFF_T. It is a drastic requirement. Do we really want to impose it on our developers? I'm asking because we can take the opposite approach, aka. fallback on a consistent naming and provide a definition to ptrdiff_t in case it is not found on the system...

[ggouaillardet]

@bosilca first, what is the rationale for having both ptrdiff_t and OPAL_PTRDIFF_TYPE ?
my initial guess is ptrdiff_t might not always be available, but OPAL_PTRDIFF_TYPE is.
assuming my guess is correct, then the portable solution is to always use OPAL_PTRDIFF_TYPE, and not build oshmem project if ptrdiff_t is not available.
if not, then btl/sm and coll/base cannot be built, which would make little sense to me.

any thoughts ?

[hjelmn]

I think the usage of OPAL_PTRDIFF_T was to support C89. Now that we require C99 and ptrdiff_t is a required type (see C99 7.17 stddef.h) we probably should remove the configury that creates OPAL_PTRDIFF_T and replace all occurances with ptrdiff_t. A compiler that doesn't have ptrdiff_t is not compliant and should not be supported.

[ggouaillardet]

sounds fair to me, I will update the PR accordingly
@PHHargrove by any chance, do you collect config.status on your so many plaforms ?
if yes, can you check ptrdiff_t is always defined ?

[PHHargrove]

@ggouaillardet
I don't "collect" any of the outputs or files from my test builds to a central location.
I just get email with a PASS or an indication of which step failed.
However, the config.status and stdout/err of configureshould still be sitting on disk on all of the platforms I've tested (except those where I am tight on disk).

I checked 25+ systems and all of them show ptrdiff_t was detected by configure.
Notably missing, however, are the results for an IA64-based Altix (down) and for Solaris (bulids removed due to storage shortage).

In principle I agree w/ Nathan that ptrdiff_t is a C99 requirement that you should not need to provide a replacement for.

[ibm-ompi]

Test passed.

[ggouaillardet]

@bosilca i updated the PR by removing OPAL_PTRDIFF_TYPE and always using ptrdiff_t

[lanl-ompi]

Test FAILed.

[bosilca]

Looks mostly as a gigantic sed. 👍

[jsquyres]

@ggouaillardet This failed several CI tests. Please check.

[ggouaillardet]

@bosilca @jsquyres i updated the PR (i basically missed the sed in the test directory)
i also split it into 3 commits, in order to isolate the "gigantic sed"

please update the Milestone, or add Target tag(s) and i will PR accordingly

[jsquyres]

Please add a Signed-off-by line to this PR's commit.

[bosilca]

@ggouaillardet any update on this issue ?

[ggouaillardet]

@jsquyres i signed-off the 3 commits
@bosilca i rebased the PR and it should be good now