Skip to content

fortran: fix common symbol sizes and alignments #13230

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ggouaillardet
Copy link
Contributor

Refs #13043

Thanks MJ Rutter for the report

@ggouaillardet
Copy link
Contributor Author

Fortran common blocks have extra alignment requirements:

  • 16 with gfortran
  • 32 with ifort
  • 64 with nvfortran
    so align the C symbols accordingly to make picky linkers (e.g. Ubuntu) happy pandas.
    Also fix types so the sizes match between C and Fortran and keep the linker happy.

Will update the commit message later.

@ggouaillardet ggouaillardet force-pushed the topic/fortran_common_alignment branch 6 times, most recently from 1f6b2fc to 5c8fbd6 Compare May 4, 2025 16:54
with mpif-h and usempi, MPI constants (e.g. MPI_COMM_WORLD)
are all parts of a unique common block. Fortran compilers
generally have alignment requirements for these (16 with gfortran,
32 with ifort or 64 with nvfortran to name a few), so pass
these requirements to the actual symbols that are defined in
the C code to make pick linkers (e.g. ubuntu) happy pandas.
Such linkers also complain about the size of MPI_STATUS, so define
these are arrays intead of pointer.

Refs open-mpi#13043

Thanks MJ Rutter for the report

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
@jsquyres
Copy link
Member

jsquyres commented May 5, 2025

Just to create a corresponding cross-link: I stole this commit and put it in #13231, and updated the Perl to Python (and fixed the missing dim in the C variable instance from the version of this commit that I copied; I think you fixed it here in a later push).

Have a look at #13231 and see what you think. We could possibly use #13231 for main / v6.0.x, and re-target this PR for v5.0.x...?

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.

2 participants