Skip to content
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

romio: implement binding generation for ROMIO #7228

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

hzhou
Copy link
Contributor

@hzhou hzhou commented Dec 1, 2024

Pull Request Description

[note: split preparation PR in #7232]

Integrate ROMIO into MPICH's binding generation framework. This will unify and simplify the IO binding tasks such as large count support, fortran support, man page support.

We'll retain the current stand-alone nature of ROMIO as much as we can in this PR. There shouldn't be any noticeable difference for users who build ROMIO separate from MPICH.

  • A refactor in ROMIO splits the top routines (e.g. MPI_File_open) into a binding layer and an implementation layer -- MPI_File_open in open.c and MPI_File_open_impl in io_impl.c
  • Building ROMIO without FROM_MPICH defined stays exactly the same
  • Building from MPICH will generate the binding layer in src/binding/c/io.c, which calls into the impl layer that is defined in ROMIO.
  • ROMIO is effectively treated the same way as an internal convenience library such as mpl.
  • We no longer need #include "mpio.h" in mpi.h
  • We no longer need worry about profiling and Fortran bindings in ROMIO
  • We no longer need maintain man pages and html docs in ROMIO

Author Checklist

  • Provide Description
    Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
  • Commits Follow Good Practice
    Commits are self-contained and do not do two things at once.
    Commit message is of the form: module: short description
    Commit message explains what's in the commit.
  • Passes All Tests
    Whitespace checker. Warnings test. Additional tests via comments.
  • Contribution Agreement
    For non-Argonne authors, check contribution agreement.
    If necessary, request an explicit comment from your companies PR approval manager.

@hzhou hzhou force-pushed the 2308_romio_binding branch 14 times, most recently from 966f859 to d00e9f7 Compare December 5, 2024 05:56
@hzhou
Copy link
Contributor Author

hzhou commented Dec 5, 2024

test:mpich/ch3/tcp ✔️

test:mpich/custom
netmod: ch4:ofi
config: mpi-abi ❌

@hzhou hzhou force-pushed the 2308_romio_binding branch 2 times, most recently from 35655c4 to 3c43546 Compare December 6, 2024 04:17
@hzhou
Copy link
Contributor Author

hzhou commented Dec 6, 2024

test:mpich/custom
netmod: ch4:ofi
config: mpi-abi ✔️

@hzhou hzhou mentioned this pull request Dec 6, 2024
4 tasks
@hzhou hzhou force-pushed the 2308_romio_binding branch from 3c43546 to 5ac180d Compare December 6, 2024 17:57
hzhou added 7 commits December 6, 2024 12:17
These looks like vendor-specific code. We can easily generate them if
needed. Remove them for now to simplify refactor.
All romio error check macros takes a myname parameter (which can be
replaced by __func__). Make MPIO_CHECK_INFO_ALL consistent.
Refactor to split interface and implementation. This prepares for
binding generations.

The binding layer, e.g. open.c, close.c, etc. will be replaced by python
generation when ROMIO is building FROM_MPICH.
Add impl functions MPI_File_f2c and MPI_File_c2f for consistency and to
avoid exposing MPIOI functions. They will be used when we generate IO
bindings from MPICH and are not used if we build ROMIO outside MPICH.
Use "goto fn_fail" instead of "goto fn_exit".

Skip MPIO_Err_return_file since we will do that in top-level routines.
We call MPI_File_delete in case there is a leftover testfile, which will
interfere with later MPI_File_open. Not checking error return and call
"delete" on a seemingly non-existent file is myterious. Add a comment
and error checking for maintenance.
@hzhou hzhou force-pushed the 2308_romio_binding branch from 5ac180d to 3c5dc12 Compare December 6, 2024 23:05
hzhou added 2 commits December 6, 2024 17:14
Need double cast to suppress warnings when converting between pointers
and integers.

Also remove an unused macro, HAVE_INT_LT_POINTER.
Define the complete MPI-IO interface in mpi.h.

In all_romio_symbols.c, we can use MPI_Request instead of MPIO_Reuqest
since MPIO_Request is not longer defined in mpi.h (since it no longer
includes mpio.h).
hzhou added 8 commits December 6, 2024 17:14
Add io_api.txt and generate io bindings
This function is now defined in src/binding/c/io_api.txt.
We don't have error checking for async_thing, amode, whence.
Wrap the code that generates c_bindings.c in a function. This prepares
for later adding io binding generations. ROMIO bindings will be thinner
than non-IO functions. So we will a separate function to generate IO
bindings.
Refactor code to function dump_body_of_routine.

This is a preparation for generating io bindings. Since ROMIO is
designed on top of the rest of MPI, we will handle differently for
parameter filtering and error checking, but the part that handles large
count and calling the internal impl functions can be shared.
MPICH will generate the binding layer.

Now that libromio just supply internal code, we no longer need worry
about libromio vs libpromio. We don't separate man pages and include
mpio.h in mpi.h either.
We no longer have any convenience libraries that need go into libmpi.so
when libpmpi.so is also built. Thus, the legacy mpi_convenience_libs
variable is no longer in use. However, we need differentiate libromio
from the other convenience libraries for the ABI build. ROMIO depends on
MPI, thus will need separate version for ABI and non-ABI build. Thus, we
create two variables -- mpi_convenience_libs and abi_convenience_libs.
Note: the former reuses the name but now with different meanings.
@hzhou hzhou force-pushed the 2308_romio_binding branch from 3c5dc12 to 06fc378 Compare December 6, 2024 23:27
hzhou added 2 commits December 6, 2024 17:59
Generate io bindings including prototypes in mpi_proto.h and
mpir_impl.h.

src/binding/c/io.c caintains all the IO binding functions that calls the
corresponding _impl functions. But it is not included in the Makefile
yet.

Dump the prototypes for io impl functions separately to allow inserting
necessary missing declarations such as MPIR_Ext_cs_enter/exit.

IO functions need use MPIR_Ext_cs_enter/exit for global cs. Now it gets
more complicated, refactor into dump_global_cs_enter/exit for better
readability.
Because ROMIO is built on top of the rest of MPI, it is difficult to use
the abi wrappers because we need either -

a. convert from ABI into MPICH abi and then convert back to ABI before
   calling into ROMIO. This seems silly.

Or -

b. build ROMIO on MPICH abi. But in libmpi_abi.so, MPICH abi is not
   available.

Thus we need -

c. Switch ROMIO to call MPICH impl functions instead of `MPI/PMPI`
   functions. We'll need to build a layer in ROMIO switch between using
   `MPI/PMPI` or using `MPIR_Xxx_impl` depending on build options.
   This will result in ROMIO having full access to MPICH internals!
   But let's delay work until we have a convincing need.

Thus, this commit implements io abi bindings without the abi wrappers.
We implemented a separate path rather than making the existing code
more complex.
@hzhou hzhou force-pushed the 2308_romio_binding branch from 06fc378 to a4c17e9 Compare December 6, 2024 23:59
@hzhou hzhou marked this pull request as ready for review December 6, 2024 23:59
@hzhou
Copy link
Contributor Author

hzhou commented Dec 6, 2024

test:mpich/ch3/tcp

test:mpich/custom
netmod: ch4:ofi
config: mpi-abi

@hzhou hzhou requested review from raffenet and roblatham00 December 9, 2024 14:01
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.

1 participant