Repository navigation
Conversation
src/api/generated_api.jl was last regenerated in 2022 from MPICH 4.0 headers, so
the bindings covered MPI 4.0. Regenerate from MPICH 5.0: 29 procedures added (the
MPI 4.1 buffer-attach, MPI_Status_get_*/set_* and MPI_Request_get_status_*
families, MPI_Type_get_value_index, MPI_Get_hw_resource_info, MPI_Remove_error_*,
and MPI 5.0's MPI_Abi_*), 2 removed.
Nothing had caught the drift: gen/ was not a trigger path for any workflow. Add a
Generator job that reruns gen.sh and diffs the result, and lint gen/gen.sh.
Make gen.sh reproducible. It followed whatever MPIPreferences the surrounding
environment resolved to -- which need not even be a supported source -- so pin the
choice in gen/LocalPreferences.toml, before Pkg runs, since MPICH_jll's platform
augmentation consults that preference at resolve time. Drop a stale gen/Manifest.toml
too: it is not committed, so it is not what makes this reproducible, and reusing one
silently generates against the wrong MPI version. Clang.jl drives the formatting of
the whole output file and only works on Julia 1.10 (it needs Clang_jll <= 15), so
pin both.
@mpichk now always probes for the symbol, not just for the procedures that happened
to carry a minimum version -- there were eight -- so the whole MPI 4.1/5.0 surface
degrades to a FeatureLevelError rather than an unresolved symbol. min_version is
Union{Nothing,VersionNumber}: procedures that predate MPI 3.1 get no version, since
MPI.jl requires MPI 3.0 anyway and there is no public machine-readable way to split
1.0 through 3.1 apart.
The version table itself is now derived rather than hand-listed, from the MPI
standard's own machine-readable bindings; see gen/versions/README.md. The same pass
yields the set of procedures that have a large-count MPI_*_c entry point.
@mpichk also gained a fallback= form, which those MPI_*_c wrappers use: where the
library predates MPI 4.0, MPI_Foo_c becomes a call to MPI_Foo, so a caller can name
one entry point on every library. A count too large for the narrow interface then
raises InexactError instead of being truncated. MPI_Op_create_c and
MPI_Register_datarep_c are excluded, because their callback's len argument widens
along with the counts and swapping the creators would read it at the wrong width.
Two fixes fall out. MPI_Aint_add and MPI_Aint_diff return an MPI_Aint, not an error
code, but were wrapped in @mpichk, so any nonzero result was thrown as an MPIError;
error checking is now derived from the ccall return type rather than a hand-kept
list. And the handle conversions are ignored rather than generated: they return a
handle, which @mpichk would likewise mistake for an errcode, and api.jl writes them
by hand.
MPICH's docs have caught up since the comment in _doc_external was written, so point
the generated man-page links at the current release rather than v4.0, which has no
pages for anything added since.
The buffer types hard-coded Cint counts, so MPI.Bcast! on an array of more than
typemax(Cint) elements died in Cint(length(arr)) -- and the 154 MPI_*_c entry points
the API layer has always exposed were called by nothing above src/api/.
Type the counts and displacements the four buffer types carry with API.Count,
API.Displ and API.TypeDispl, and route the 54 affected call sites to the MPI_*_c
entry points. Those aliases follow the MPI library: MPI_Count/MPI_Aint/MPI_Count
where it provides the large-count entry points, Cint/Cint/MPI_Aint where it does
not, in which case the wrappers fall back to the narrow entry points and the widths
line up. So large messages work with no special handling on MPI 4.0 or later, and
raise InexactError rather than truncating on anything older.
Three aliases and not one because the widenings genuinely differ: MPI_Alltoallv_c
widens its counts to MPI_Count but its displacements only to MPI_Aint, while
MPI_Type_create_struct_c widens its byte displacements all the way to MPI_Count.
The three coincide on 64-bit ABIs and come apart on 32-bit ones.
Left alone deliberately:
- Ranks, roots, tags, keyvals, flags, request indices and everything in topology.jl
are still int in MPI 5.0 -- the absence of _c variants for MPI_Cart_create,
MPI_Dist_graph_* and MPI_Waitsome says so.
- User-defined reduction operators keep using the narrow MPI_Op_create. Which
callback signature applies is fixed by the creator, not by the reduce entry point
consuming the handle, and OpWrapper's loop is element-wise, so an implementation is
free to chunk a large-count reduction into calls the int *len callback can express.
test_largecount.jl checks that, behind JULIA_MPI_TEST_LARGECOUNT.
Two behaviour changes worth knowing about. A VBuffer built from a Vector{Cint} now
copies rather than aliasing the caller's array, because the conversion is no longer
the identity. And MPI.bcast of an arbitrary object broadcasts its serialized length
as Int64 rather than Cint -- part of the wire format, so fixed rather than
build-dependent, and it lifts the 2 GiB cap that Cint imposed.
UBuffer.nchunks becomes Int: it is never passed to MPI, being only validated against
Comm_size, yet narrowing it through Cint meant UBuffer(arr, 1) threw InexactError for
an array of more than typemax(Cint) elements.
The GPU extensions now let Buffer convert the count instead of writing Cint
themselves, so they stay out of the way of any future change of this kind.
Drop the Generator workflow. Regenerating is a deliberate act -- it decides which MPI standard version the bindings cover -- and its diff wants reading rather than enforcing, so it does not belong in CI. That makes gen/gen.sh the only entry point, so put the instructions in it: which Julia it needs and why, how to choose the implementation whose headers get read, where the MPI standard version is actually decided, what to look for in the diff afterwards, and when src/versions.jl needs refreshing alongside. The ShellCheck workflow keeps linting gen/gen.sh. That checks the script, it does not run the generator, and the script matters more now that it is the documented way in.
Mostly prose. Two things behind it were actually broken: The banner in src/api/generated_api.jl named `MPI.jl/.claude/worktrees/<name>/gen/src/MPIgenerator.jl`, because the path was derived as `replace(@__FILE__, r".*MPI.jl" => "MPI.jl")`, which keeps everything after the *last* `MPI.jl` in the path -- and in a git worktree that is the worktree subpath. Whoever regenerated leaked their checkout into a committed file. Derive it relative to the repository root instead. gen/versions/extract_versions.py ignored an include directory that did not exist, so a garbage-collected MPICH_jll artifact made it silently classify every MPI-4.0 procedure as new in 4.0 -- 662 entries instead of 250, and no complaint. Make it exit when it finds no declarations. Both generated files are regenerated here, so they match their generators again: gen/src/versions.jl had also drifted, carrying wording the script no longer emits.
Contributor
Author
|
Closes #880. |
Member
|
Do we expect performance impact? |
CI caught this on Intel oneAPI MPI 2021.11: a `Ref{Int64}` reaching a `Ptr{Cint}`
inside `MPI_Type_size`, via `MPI_Type_size_c`.
That library provides `MPI_Send_c` but not `MPI_Type_size_c`. `HAS_LARGE_COUNT`
probed a single symbol, so it reported true and made `API.Count` an `Int64`, while
`@mpichk` decided the fallback per function and sent `MPI_Type_size_c` to the narrow
entry point, which wants a `Ptr{Cint}`. Counts passed by value survive that, since
`ccall` converts them; counts passed through a pointer do not.
The two decisions have to agree, and the count types are necessarily one choice for
the whole package -- a `VBuffer`'s counts feed several different collectives. So make
the fallback all-or-nothing instead: the generator emits the large-count symbols it
wrapped as `LARGE_COUNT_SYMBOLS`, `HAS_LARGE_COUNT` is true only when the library
provides every one of them, and `@mpichk` falls back on that flag rather than on each
symbol's own availability. A library shipping part of the set now uses none of it.
Verified against three configurations: MPICH_jll, which has all of them; OpenMPI_jll,
which has none; and MPICH_jll with one symbol removed from the list, which reproduces
Intel's shape -- large-count entry points present in the library, none of them used.
The suite passes in all three, and the new assertion in test_largecount.jl fails
whenever `HAS_LARGE_COUNT` disagrees with what the library actually exports.
The 32-bit CI jobs failed in test_largecount.jl:
MPI_Type_contiguous_c(count=2147483648, MPI_BYTE, ...) failed
The input argument count is too big to fit for internal routines
Displacements and datatype extents are `MPI_Aint`, which is pointer-sized. So on a
32-bit build nothing can describe 2 GiB however wide `MPI_Count` is, and a 32-bit
address space could not hold such a buffer anyway. `HAS_LARGE_COUNT` is about which
entry points exist, which is not the same question.
Gate the parts of the test that need a 2 GiB extent, displacement or buffer on
whether `MPI_Aint` can address one, and say so in the docs, since it decides whether
large counts buy anything at all. Where the entry points are present but the address
space is too small, require the datatype constructor to fail cleanly rather than
silently truncate -- both 32-bit configurations in CI raise `MPIError` there.
Member
|
Also, it goes without saying that I'm not going to review very easily a 3k lines PR. Separating changes into different PRs may help a lot. For example, the first commit is just updating the autogenerated bindings, right? That can easily go into its own PR, and then you can stack the rest on top of it |
Contributor
Author
|
Performance impact: No, I do not expect a performance impact, except in one case:
|
Contributor
Author
|
I have begun to split this PR into several, starting with #990 for the API regeneration. |
Contributor
Author
|
#991 now contains the second half of this PR. I will close this PR now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a large PR. It makes MPI.jl use the large-count MPI functions if available (MPI 4.0 or later).
Update the API generator to handle MPI 4.0, 4.1, and 5.0. This is mainly a mechanical change.
The large-count function (e.g.
MPI_Send_c) take a 64-bit count argument whereas the narrow functions (e.g.MPI_Send) only take a 32-bit count. On a 64-bit system, 64-bit counts are clearly desirable.To handle older MPI libraries which do not provide the large-count functions, I decided to do this in the code generator: The large-count functions (
MPI_Send_c) are always provided. If the MPI library provides them, they are called. If not – and that is known at precompile time – then the narrow function (MPI_Send) is called instead. This means that the high-level functions do not need to care about this distinction, they can always call the large-count functions. The type conversion for counts (fromIntto eitherInt64orCint) are handled by@ccall, efficiently, without a run-time check. Using a count argument that is too large for the MPI library throws an error.There are a few other minor details that needed sorting out, most described in comments in the new code. A nice surprise was that user-defined reduction operators continue to work as-is: We can continue to define the user-registered callbacks with narrow count arguments (
Cint), and MPI implementations then automatically chunk large reduction operations.