Skip to content

coll/libnbc: correctly handle MPI_BOTTOM #9651

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bosilca merged 1 commit into from
Nov 28, 2021
Merged

coll/libnbc: correctly handle MPI_BOTTOM #9651

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ompi/mca/coll/libnbc/nbc_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
*
* Copyright (c) 2012 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2014 NVIDIA Corporation. All rights reserved.
* Copyright (c) 2015-2018 Research Organization for Information Science
* Copyright (c) 2015-2021 Research Organization for Information Science
* and Technology (RIST). All rights reserved.
* Copyright (c) 2015 Los Alamos National Security, LLC. All rights
* reserved.
Expand Down Expand Up @@ -565,7 +565,7 @@ static inline void NBC_SchedCache_dictwipe(hb_tree *dict_in, int *size) {
#define NBC_IN_PLACE(sendbuf, recvbuf, inplace) \
{ \
inplace = 0; \
if(recvbuf == sendbuf) { \
if(recvbuf == sendbuf && MPI_BOTTOM != sendbuf) { \
Copy link
Member

@bosilca bosilca Nov 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this test really valid ? The MPI standard prohibits using a recvbuf equal to the sendbuf to indicate an IN_PLACE operation. Why is this macro accepting it ?

Copy link
Contributor Author

@ggouaillardet ggouaillardet Nov 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get your point, but are you 100% sure about this?

Bad things will definitely occurs if sendbuf == recvbuf and for example the same datatype is used.
But if the user pass sendbuf == recvbuf with for example two datatypes that describes non overlapping subarrays, would that violate the standard? if yes, is there an explicit exception for MPI_BOTTOM ?
Also, should we enhance the C bindings and error out with a user friendly error code if the user passes sendbuf == recvbuf (unless MPI_BOTTOM) ?

Copy link
Member

@bosilca bosilca Nov 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, the standard prohibit overlapping regions between the typemap of the send and receive buffers. So, while it is legal to use sendbuf == recvbuf it can never represent an IN_PLACE operation. So, that macro seems a little extreme in the detection of IN_PLACE.

inplace = 1; \
} else \
if(sendbuf == MPI_IN_PLACE) { \
Expand Down