Skip to content

Conversation

@markalle
Copy link
Contributor

In the below type creation sequence
MPI_Type_create_resized(MPI_INT, 0, 6, &mydt1);
MPI_Type_contiguous(1, mydt1, &mydt2);
I think both mydt1 and mydt2 should have extent 6.

The Type_create_resized would add an UB marker into the type map,
and the definition of Type_contiguous would maintain the same
markers in the new map.

The only counter argument I can think of to the above is if
we declared that mydt1 is illegal because it's putting data
on addresses that don't satisfy the alignment requirement.

But in my interpretation of the standard the term "alignment
requirement" is a property of the system memory, and MPI defines
"extent" in a way to make it easy to create MPI datatypes that
support the system's alignment requirements. But the standard
isn't saying it's illegal to make MPI datatypes that don't satisfy
the system's alignment requirements. I think this is true also
because the MPI datatypes might be used in file IO where the
requirements are different, so that's my long winded explanation
for why I don't think we can declare mydt1 illegal.

Complete example:
#include <stdio.h>
#include <mpi.h>
int main() {
MPI_Datatype mydt1, mydt2;
MPI_Aint lb, ext;
MPI_Init(0, 0);
MPI_Type_create_resized(MPI_INT, 0, 6, &mydt1);
MPI_Type_commit(&mydt1);
MPI_Type_contiguous(1, mydt1, &mydt2);
MPI_Type_commit(&mydt2);

    MPI_Type_get_extent(mydt1, &lb, &ext);
    printf("mydt1 extent %d\n", (int)ext);
    MPI_Type_get_extent(mydt2, &lb, &ext);
    printf("mydt2 extent %d\n", (int)ext);

    MPI_Type_free(&mydt1);
    MPI_Type_free(&mydt2);
    MPI_Finalize();
    return(0);
}

% mpicc -o x test.c
% mpirun -np 1 ./x
Without this PR the output is

mydt1 extent 6
mydt2 extent 8
With this PR both extents are 6.

Fwiw I also tested with mpich and they give 6 for both extents.

Signed-off-by: Mark Allen [email protected]
(cherry picked from commit bca3c0e)

In the below type creation sequence
    MPI_Type_create_resized(MPI_INT, 0, 6, &mydt1);
    MPI_Type_contiguous(1, mydt1, &mydt2);
I think both mydt1 and mydt2 should have extent 6.

The Type_create_resized would add an UB marker into the type map,
and the definition of Type_contiguous would maintain the same
markers in the new map.

The only counter argument I can think of to the above is if
we declared that mydt1 is illegal because it's putting data
on addresses that don't satisfy the alignment requirement.

But in my interpretation of the standard the term "alignment
requirement" is a property of the system memory, and MPI defines
"extent" in a way to make it easy to create MPI datatypes that
support the system's alignment requirements.  But the standard
isn't saying it's illegal to make MPI datatypes that don't satisfy
the system's alignment requirements.  I think this is true also
because the MPI datatypes might be used in file IO where the
requirements are different, so that's my long winded explanation
for why I don't think we can declare mydt1 illegal.

Complete example:
    #include <stdio.h>
    #include <mpi.h>
    int main() {
        MPI_Datatype mydt1, mydt2;
        MPI_Aint lb, ext;
        MPI_Init(0, 0);
        MPI_Type_create_resized(MPI_INT, 0, 6, &mydt1);
        MPI_Type_commit(&mydt1);
        MPI_Type_contiguous(1, mydt1, &mydt2);
        MPI_Type_commit(&mydt2);

        MPI_Type_get_extent(mydt1, &lb, &ext);
        printf("mydt1 extent %d\n", (int)ext);
        MPI_Type_get_extent(mydt2, &lb, &ext);
        printf("mydt2 extent %d\n", (int)ext);

        MPI_Type_free(&mydt1);
        MPI_Type_free(&mydt2);
        MPI_Finalize();
        return(0);
    }
% mpicc -o x test.c
% mpirun -np 1 ./x
Without this PR the output is
> mydt1 extent 6
> mydt2 extent 8
With this PR both extents are 6.

Fwiw I also tested with mpich and they give 6 for both extents.

Signed-off-by: Mark Allen <[email protected]>
(cherry picked from commit bca3c0e)
@awlauria awlauria changed the title make Type_create_resized set FLAG_USER_UB v4.1.x: make Type_create_resized set FLAG_USER_UB Oct 19, 2020
@bwbarrett bwbarrett merged commit b3487eb into open-mpi:v4.1.x Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants