Skip to content

Bring better Fortran TKR support in 4.1 #13245

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

Jump to bottom
Merged
merged 7 commits into from
May 20, 2025
Merged

Bring better Fortran TKR support in 4.1 #13245

merged 7 commits into from
May 20, 2025

Conversation

bosilca
Copy link
Member

@bosilca bosilca commented May 9, 2025 •
edited
Loading

This PR updates the 4.1 with #12512 and some parts of 30eb36f and b6017d2.

bot:notacherrypick

@bosilca bosilca added this to the v4.1.9 milestone May 9, 2025
@bosilca bosilca requested a review from janjust May 9, 2025 21:05
@bosilca bosilca added the bug label May 9, 2025
@open-mpi open-mpi deleted a comment from github-actions bot May 9, 2025
@janjust
Copy link
Contributor

janjust commented May 9, 2025

bot:retest

@jsquyres
Copy link
Member

There's a legit make distcheck CI failure that needs to be fixed.

Is there a reason to bring this back to v4.1.x? We're not planning on any future 4.1.x releases unless we really, really need to.

@bosilca
Copy link
Member Author

bosilca commented May 13, 2025 •
edited
Loading

I don't understand how this works on main and 5.0 but not on the 4.1 ! The root issue is that one of the files generated during configure, the check_ignore_tkr.mod file generated from config/ompi_fortran_check_ignore_tkr.m4:202, is left behind after make dist. The m4 files are identical and I can't find anything that clean that file on main/5.x.

I see in the configure script that we remove all conftest.* files, but the compiler will generate a mod file based on the module name, aka. check_ignore_tkr.mod, and this file is never removed in any OMPI versions I see. How is this file supposed to get removed ? And why that mechanism does not work in the 4.1 ?

@jsquyres jsquyres mentioned this pull request May 16, 2025
Merged
@jsquyres
Copy link
Member

@bosilca See #13255

@bosilca
Copy link
Member Author

bosilca commented May 16, 2025

For completeness let me explain what was happening here. It is a combination of two things:

  1. I missed a 4 years old commit (c08258c) that was cleaning the .mod files after OMPI_FORTRAN_CHECK_BIND_C_TYPE. As a result, the 4.1 did not benefit from the same side-effect, aka. cleaning unrelated .mod files (in a unrelated function).
  2. c08258c adds the missing cleanup, so now everything should be properly removed.

@bwbarrett
Copy link
Member

bot:aws:retest

@jsquyres jsquyres force-pushed the topic/4.1/add_support_for_LLVM19 branch from 6fc2c88 to b57cbfa Compare May 19, 2025 22:18
@jsquyres
Copy link
Member

Rebased to tip of v4.1.x to include CI updates.

ggouaillardet and others added 7 commits May 19, 2025 18:53
@ggouaillardet @jsquyres
configury: harden Fortran IGNORE_TKR check
1308515 
as reported in open-mpi#12506, upcoming LLVM 19 can generate some
false positive that will make the mpi Fortran modules unusable.
Harden the test by using a module in order to fix that.

Thanks Paul Osmialowski for bringing this to our attention.

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
(cherry picked from commit 8c601e1)
@ggouaillardet @jsquyres
configury: add Fortran IGNORE_TKR check for LLVM 19
466c737 
Try the !DIR$ IGNORE_TKR directive and the type(*)
type in order to support upcoming LLVM 19 flang compiler.

Thanks Paul Osmialowski for the report and suggesting the solution.

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
(cherry picked from commit 0bc995f)
@ggouaillardet @jsquyres
configury: harden Fortran ASYNCHRONOUS check
c74a05c 
LLVM 18 supports the ASYNCHRONOUS keyword in an interface,
but not in a subroutine (!) so test both in order to avoid
a false positive that will lead to compilation failure.

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
@ggouaillardet @jsquyres
configury: harden IGNORE_TKR check
1418173 
NVIDIA HPC Compiler (e.g. nvfortran) incorrectly selected the GCC
style pragmas to support IGNORE_TKR. Harden the test by mimicking
exactly the mpi f08 bindings in order to fix that false positive.

Thanks Chris Parrot for the report.

Refs. open-mpi#11582

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
(cherry picked from commit 600df6a)
@bosilca @jsquyres
Import the Fortran part of 30eb36f
8f599a9 
Signed-off-by: George Bosilca <gbosilca@nvidia.com>
@ggouaillardet @jsquyres
configury: cleanup .mod file
cc06926 
manually cleanup the generated .mod file in OMPI_FORTRAN_CHECK_BIND_C_TYPE

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
(cherry picked from commit 466a2b3)
@jsquyres
configury: ensure to remove *.mod files
a189f1f 
At the end of the OMPI_FORTRAN_CHECK_IGNORE_TKR_SUB macro, ensure to
remove any generated *.mod files (rather than relying on some other
macro -- cough cough OMPI_FORTRAN_CHECK_BIND_C_TYPE cough cough -- to
remove *.mod files for us).

Signed-off-by: Jeff Squyres <jeff@squyres.com>
(cherry picked from commit c08258c)
@jsquyres jsquyres force-pushed the topic/4.1/add_support_for_LLVM19 branch from b57cbfa to a189f1f Compare May 19, 2025 22:54
@jsquyres jsquyres merged commit 37e1204 into open-mpi:v4.1.x May 20, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsquyres jsquyres jsquyres approved these changes

@janjust janjust Awaiting requested review from janjust

Assignees
No one assigned
Labels
bug Target: v4.1.x
Projects
None yet
Milestone
v4.1.9
Development

Successfully merging this pull request may close these issues.
5 participants
@bosilca @janjust @jsquyres @bwbarrett @ggouaillardet