Skip to content

Fortran logical fix. #9405

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.

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
merged 1 commit into from
Sep 23, 2021
Merged

Conversation

awlauria
Copy link
Contributor

I don't have a testcase for this, I just found the bug while inspecting
OMPI's use of OMPI_FORTRAN_VALUE_TRUE. The following line:
if (OMPI_FORTRAN_VALUE_TRUE == *flag) ...
is okay in the model where users build a whole new
OMPI depending on what fortran compiler they're using. But for a general
purpose build, it requires OMPI_LOGICAL_2_INT(), which
converts the incoming fortran-logical to 0 or 1 for C, eg
if (1 == OMPI_LOGICAL_2_INT(*flag)) ...

Signed-off-by: Austen Lauria [email protected]

I don't have a testcase for this, I just found the bug while inspecting
OMPI's use of OMPI_FORTRAN_VALUE_TRUE. The following line:
   if (OMPI_FORTRAN_VALUE_TRUE == *flag) ...
is okay in the model where users build a whole new
OMPI depending on what fortran compiler they're using. But for a general
purpose build, it requires OMPI_LOGICAL_2_INT(), which
converts the incoming fortran-logical to 0 or 1 for C, eg
    if (1 == OMPI_LOGICAL_2_INT(*flag)) ...

Signed-off-by: Austen Lauria <[email protected]>
@awlauria awlauria requested a review from jsquyres September 22, 2021 14:25
@awlauria awlauria mentioned this pull request Sep 22, 2021
@awlauria awlauria requested a review from markalle September 22, 2021 14:28
@jsquyres jsquyres merged commit 72d3006 into open-mpi:master Sep 23, 2021
@awlauria awlauria deleted the fortran_logic_master branch September 23, 2021 12:39
@awlauria
Copy link
Contributor Author

@jsquyres should I bring this back to the v4 series?

@jsquyres
Copy link
Member

I don't have strong feelings; I think the use case for it is mostly baked into Spectrum and not others.

I certainly don't object if you want to bring it back to v4.0.x and v4.1.x for completeness: it's correct, regardless of the specific use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants