Skip to content

Conversation

@hzhou
Copy link
Contributor

@hzhou hzhou commented Jan 4, 2026

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.

Add MPIX_{Comm,Type,Win}_create_keyval_x similar to src/binding/fortran/mpif_h/user_proxy.c. Although MPI_{Comm,Type,Win}_create_keyval has an extra_state for context, but it does not have a destructor callback, which made any context that require cleanup unusable

Note: a destructor callback is necessary if we want allow user to be able to clean up the extra_state even without the consideration for MPI ABI or language bindings. The extra_state is associated with KEYVAL, which is internally reference counted and garbage collected. Without a destructor callback, user won't be able to cleanly destroy the extra_state.

[skip warnings]

Reference: #6965

Attribute semantics across languages

C

int MPI_Comm_set_attr(MPI_Comm comm, int comm_keyval, void *attribute_val);
int MPI_Comm_get_attr(MPI_Comm comm, int comm_keyval, void *attribute_val, int *flag);

But the signature is misleading. The attribute_val in _set_attr is the value of attribute, but in _get_attr is the address of attribute (as value out).

Typical C style. No problem.

Fortran

MPI_COMM_SET_ATTR(COMM, COMM_KEYVAL, ATTRIBUTE_VAL, IERROR)
    INTEGER(KIND=MPI_ADDRESS_KIND) ATTRIBUTE_VAL
    
MPI_COMM_GET_ATTR(COMM, COMM_KEYVAL, ATTRIBUTE_VAL, FLAG, IERROR)
    INTEGER(KIND=MPI_ADDRESS_KIND) ATTRIBUTE_VAL    

Fortran doesn't have the signature difference between _set and _get, always an INTEGER. No problem.

C _set + Fortran _get

Fortran will get an INTEGER. The question is how C sets the integer.

** main branch

int a = 10;
MPI_Comm_set_attr(comm, comm_keyval, 10);

** This PR

MPI_Aint *p_a = malloc(sizeof(MPI_Aint));
*p_a = 10;
MPI_Comm_set_attr(comm, comm_keyval, p_a);

** Alternative
Same as main branch

Fortran _set + C _get

** main branch

int a, flag;
MPI_Aint *p_a;
MPI_Comm_get_attr(comm, comm_keyval, &p_a, &flag);
if (flag) {
    a = *p_a;
}

** This PR
Same

** Alternative

int flag;
MPI_Aint a;
MPI_Comm_get_attr(comm, comm_keyval, &a, flag);

Issue

In old MPICH, MPI_Comm_get_attr returns value or address to a value by remember who sets the attribute and who gets the attribute.

Using standard MPI ABI, there is no mechanism for MPICH to tell which binding sets and which binding gets.

Which interpretation is more sensible

  • Who didn't find the requirement of passing in a pointer to a pointer to an int in order to get an int strange?
  • In the new interpretation, an attribute is a scalar, period. If user choose to store a pointer as scalar, perfectly fine.
  • There is nothing we can do with the established convention of retrieving builtin attributes. Bindings can deal with it as long as it can tell whether a keyval is builtin.

Does this matter

Attributes is for caching (within a design).

Is there any real usage that use attributes across language bindings?

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 2601_fort_attr branch 9 times, most recently from d0fa5f0 to 86cce74 Compare January 8, 2026 16:52
hzhou added 6 commits January 8, 2026 16:50
Add MPIX_{Comm,Type,Win}_create_keyval_x.

Similar to MPI_{Comm,Type,Win}_create_keyval, but with an additional
destructor callback so the users can cleanup the extra_state.
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.
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.
@hzhou hzhou force-pushed the 2601_fort_attr branch 4 times, most recently from 4df1c45 to 53f6e04 Compare January 9, 2026 15:26
hzhou added 4 commits January 9, 2026 09:38
Use MPIX_{Comm,Type,Win}_create_keyval_x so we can clean up the
extra_state.

The previous code that frees proxy state in MPI_Keyval_free is incorrect
because the keyval may still be in use after MPI_Keyval_free. With the
new API, we free the state in the destructor callback. We no longer need
MPII_Keyval_free etc and we no longer need uthash to look up the proxy
state from keyval.
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 old implementation let C directly set integer as scalar, but will
return integer pointer in _get_attr *if* _set_attr is from Fortran.

There is no way to keep the same behavior without C implementation
keeping track of the language binding in both _set_attr and _get_attr.

This commit implements one alternative. It keeps the C _set_attr
behavior, but alters the C _get_attr behavior. C _get_attr will directly
return the scalar value instead of a pointer.

The tricky case is with builtin keyvals such as MPI_TAG_UB. The standard
specifies the return in _get_attr should be a pointer to an int. We
conform to this specification by checking whether a keyval is builtin in
the fortran binding.
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.
@hzhou hzhou force-pushed the 2601_fort_attr branch 5 times, most recently from d4859d9 to 009dfa5 Compare January 9, 2026 16:43
hzhou added 2 commits January 9, 2026 11:22
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.
This commit implements the second alternative, overwriting the earlier
commit that implements the first alternative.

Fortran bindings will allocate an MPI_Aint internally and convert in
MPI_Xxx_set_attr functions from INTEGER to a pointer to MPI_Aint before
calling the C correspondent. And vice versa for MPI_Xxx_get_attr
functions.

This alternative keeps C _get_attr behavior, but changes the C _set_attr
behavior (only if it intends to _get_attr in Fortran).
hzhou added 4 commits January 9, 2026 11:22
We also need modify the tests if we force fortran to always store
pointers. The C _set_attr cannot store scalars if it intends for Fortran
to read.
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.
@hzhou
Copy link
Contributor Author

hzhou commented Jan 9, 2026

test:mpich/ch3/most
test:mpich/ch4/most

@hzhou hzhou requested a review from raffenet January 9, 2026 17:50
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