binding/fortran: remove dependency on MPII functions #7700
+445
−804
Conversation
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
hzhou
force-pushed
the
2601_fort_attr
branch
5 times, most recently
from
129f82b to
de278de
Compare
Compiler complains without cast even if MPI_Fint is the same as MPI_Aint.
Use MPL_malloc (and MPL_free) instead of malloc (and free). Simplify MPII_Comm_create_errhandler etc. by reusing MPII_errhan_create.
hzhou
force-pushed
the
2601_fort_attr
branch
2 times, most recently
from
e433bcd to
6fcc5f5
Compare
The debug class MPIR_DBG_STRING is only used in MPL but defined in MPICH. This creates a link issue when MPL is used outside mpich, for example, libmpifort. Since the current usage is very nonessential - only used in MPL_str_add_string_arg in ch3, remove for now. A better debug message or logging should be provided at upper layer upon receiving the MPL error.
Use proxy functions within the fortran binding for grequest callbacks rather than requiring parameter conversions within the C impl.
Remove the dependency on non-standard MPII_Keyval_set_f90_proxy. We handle the fortran param conversions completely inside the fortran binding instead. Unlike user op and errhandler, the attr callback function accepts an extra_state context, which allows a generic binding proxy similar to that with MPIX_Op_create_x and MPIX_Comm_create_errhandler_x.
Handle the attrType conversions within the fortran binding instead of
relying on nonstandard MPII_{Comm,Type,Win}_{get,set}_attr, which use an
extra parameter MPIR_Attr.
The tricky case is with builtin keyvals such as MPI_TAG_UB. In C,
builtin keyval returns as a pointer to an internal int variable rather
than passing directly as value. With user keyval, we always can assume
the attr_value is a value cast as (void *) or MPI_Aint.
There is no good way to test whether a keyval is builtin. For now, we
hack it assuming known patterns for builtin keyvals.
The builtin keyval constants are shifted by 1 to pass to the C impl for Fortran handling. This mechanism is no longer needed now that we handle the Fortran conversions within the binding.
In creation routines that involve registering Fortran callbacks,
including MPI_{Comm,Win,File,Session}_create_errhandler,
MPI_Grequest_start, MPI_{Comm,Type,Win}_create_keyval, and
MPI_Op_create, call corresponding internal functions from
mpif_h/user_proxy.c that uses internal proxy callbacks for parameter
conversions.
In MPI_{Comm,Win}_get_attr, call MPIR_builtin_attr_c2f for post
conversions when the C interface returns an integer pointer for builtin
keyvals.
Remove the corresponding old interfaces.
Now the fortran binding will handle attr value conversions, we no longer
need pass and store attrType in the C impl.
Remove MPII_{Comm,Win,Type}_{set,get}_attr.
Remove the no longer needed fortran callback interface for grequest.
It appears the old interpretation of how we store an integer value in attribute is to store a pointer (address) to an integer. This creates the ridiculous case when we retrieve a attribute value set in Fortran, we need *guess* whether the integer attr is set in F77 or F90 and cast the returned attr from (void *) to either (int *) or (MPI_Aint *). In an alternate interpretation, we simply store an integer as a scalar. If we retrieve the integer from C, simply cast the (void *) into an integer. Integer cast is always safe! Simply, the new interpretation always treats an attribute as a scalar value (including ptr address). There is no way to implement the old interpretation with a C MPI ABI. Note, the new interpretation only affects applications that want to retrieve an attribute set in Fortran but get in C. I don't think we have any real applications involving such case.
Contributor
Author
|
test:mpich/ch3/tcp |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Pull Request Description
Remove the dependency on non-standard
MPII_Keyval_set_f90_proxy. We handle the fortran param conversions completely inside the fortran binding instead.[skip warnings]
Author Checklist
Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
Commits are self-contained and do not do two things at once.
Commit message is of the form:
module: short descriptionCommit message explains what's in the commit.
Whitespace checker. Warnings test. Additional tests via comments.
For non-Argonne authors, check contribution agreement.
If necessary, request an explicit comment from your companies PR approval manager.