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

specify values of compile-time constants #1

Open
jeffhammond opened this issue Nov 18, 2022 · 26 comments
Open

specify values of compile-time constants #1

jeffhammond opened this issue Nov 18, 2022 · 26 comments

Comments

@jeffhammond
Copy link
Member

jeffhammond commented Nov 18, 2022

Problem

We must specify the values of compile-time constants to have an ABI.

Proposal

These must be compile-time constants. We need to figure out a safe upper bound.

MPI_MAX_PROCESSOR_NAME 
MPI_MAX_LIBRARY_VERSION_STRING 
MPI_MAX_ERROR_STRING 
MPI_MAX_DATAREP_STRING 
MPI_MAX_INFO_KEY 
MPI_MAX_INFO_VAL 
MPI_MAX_OBJECT_NAME
MPI_MAX_PORT_NAME

Of the following, only MPI_BSEND_OVERHEAD is nontrivial, because it relates to implementation details.

MPI_PROC_NULL
MPI_ANY_SOURCE
MPI_ANY_TAG 
MPI_UNDEFINED 
MPI_BSEND_OVERHEAD 
MPI_KEYVAL_INVALID 
MPI_LOCK_EXCLUSIVE 
MPI_LOCK_SHARED 
MPI_ROOT

All error constants in A.1.1.

These will change with every release of the MPI standard:

MPI_VERSION
MPI_SUBVERSION

Standardizing the Fortran ABI is a different issue, but if we go down that path, these need to be specified:

MPI_STATUS_SIZE (Fortran only)
MPI_ADDRESS_KIND (Fortran only)
MPI_COUNT_KIND (Fortran only)
MPI_INTEGER_KIND (Fortran only)
MPI_OFFSET_KIND (Fortran only) 

This is a C constant that must match Fortran.

MPI_F_STATUS_SIZE (C only)

These are currently compiler-specific, in addition to being implementation-specific. However, by the time we finish MPI-5, the associated Fortran 2018 features should be widely available, and we may be able to deprecate them. Alternatively, these can become runtime queries, although that may impact user experience in some cases.

MPI_SUBARRAYS_SUPPORTED (Fortran only) 
MPI_ASYNC_PROTECTS_NONBLOCKING (Fortran only)

Changes to the Text

TODO

Impact on Implementations

Implementations will need to change to the standard values.

Impact on Users

ABI standardization is good.

References and Pull Requests

mpi-forum#642

@besnardjb
Copy link

besnardjb commented Nov 18, 2022

Maybe not fully compile time constants but almost, we need to be careful of potential "Macros":

2.6.4:

An implementation is allowed to implement MPI_WTIME, PMPI_WTIME, MPI_WTICK, PMPI_WTICK, MPI_AINT_ADD, PMPI_AINT_ADD, MPI_AINT_DIFF, PMPI_AINT_DIFF, and the handle-conversion functions (MPI_Group_f2c, etc.) in Section 19.3.4, and no others, as macros in C.

  • If MPI_Wtime calls in _mpi_get_timer() we are not portable anymore;
  • Same is for c2f handle conversion if it does some bit-shifting magic it will break;

@jeffhammond
Copy link
Member Author

Can you create a new issue for this? It is an important topic to address.

@cniethammer
Copy link

The following table summarizes what I found with respect to specifications about allowed values for the MPI_MAX_* constants in the standard:

constant at least at most MPI 4.0 ref. comment
MPI_MAX_INFO_KEY 32 255 p.479
MPI_MAX_STRINGTAG_LEN 63 p.344
MPI_MAX_OBJECT_NAME 64 p.382
MPI_MAX_PROCESSOR_NAME p.455 provided example "processor 9 in rack 4 of mpp.cs.org" expects at least 36
MPI_MAX_PSET_NAME_LEN 63 p.506
MPI_MAX_PORT_NAME p.545 not clear - IPv4:PORT, IPv6:PORT, IB GUID/LID ...? " ... network address, encoded in the port_name string, at which the server will be able to accept connections from clients the server will be able to accept connections from clients."
MPI_MAX_DATAREP_STRING 64 p.705

@jeffhammond
Copy link
Member Author

I will collect the values from all accessible implementations and tabulate, since that will guide us.

@jeffhammond
Copy link
Member Author

Intel MPI 2021.7.1

$ grep MPI_MAX_ /opt/intel/oneapi/mpi/2021.7.1/include/mpi.h
#define MPI_MAX_PROCESSOR_NAME 128
#define MPI_MAX_LIBRARY_VERSION_STRING 8192
#define MPI_MAX_ERROR_STRING   512
#define MPI_MAX_PORT_NAME      256
#define MPI_MAX_OBJECT_NAME    128
#define MPI_MAX_INFO_KEY       255
#define MPI_MAX_INFO_VAL      1024

MPICH 4.0.3

% grep MPI_MAX_ /opt/homebrew/Cellar/mpich/4.0.3/include/mpi.h
#define MPI_MAX_PROCESSOR_NAME 128
#define MPI_MAX_LIBRARY_VERSION_STRING 8192
#define MPI_MAX_ERROR_STRING   512
#define MPI_MAX_PORT_NAME      256
#define MPI_MAX_OBJECT_NAME    128
#define MPI_MAX_STRINGTAG_LEN  256
#define MPI_MAX_PSET_NAME_LEN  256
#define MPI_MAX_INFO_KEY       255
#define MPI_MAX_INFO_VAL      1024

Open-MPI 4.1.4

/opt/homebrew/Cellar/open-mpi/4.1.4_2/bin/mpicc print-compile-time-constants.c && ./a.out
MPI_VERSION=3
MPI_SUBVERSION=1
MPI_MAX_PROCESSOR_NAME=256
MPI_MAX_LIBRARY_VERSION_STRING=256
MPI_MAX_ERROR_STRING=256
MPI_MAX_DATAREP_STRING=128
MPI_MAX_INFO_KEY=36
MPI_MAX_INFO_VAL=256
MPI_MAX_OBJECT_NAME=64
MPI_MAX_PORT_NAME=1024

@jeffhammond
Copy link
Member Author

Value MPICH Open-MPI
MPI_MAX_PROCESSOR_NAME 128 256
MPI_MAX_LIBRARY_VERSION_STRING 8192 256
MPI_MAX_ERROR_STRING 512 256
MPI_MAX_PORT_NAME 256 1024
MPI_MAX_OBJECT_NAME 128 64
MPI_MAX_STRINGTAG_LEN 256 N/A
MPI_MAX_PSET_NAME_LEN 256 N/A
MPI_MAX_INFO_KEY 255 36
MPI_MAX_INFO_VAL 1024 256

@cniethammer
Copy link

In Open MPI '5.0.x' and 'main' the defaults in the current source code are

constant default value
MPI_MAX_PSET_NAME_LEN 512
MPI_MAX_STRINGTAG_LEN 1024

@jedbrown
Copy link

Could any of the current compile-time constants become link-time constants or init-time constants?

@jeffhammond
Copy link
Member Author

jeffhammond commented Dec 13, 2022

Anything that is not used for a static string declaration can be unspecified at compile-time. See MPI 4.0 Section 2.5.4 for details.

Fortran requires compile-time constants for initialization but Fortran isn't the top priority here. However, the MPICH ABI approach is much better for Fortran.

@jedbrown
Copy link

Yeah, I was mostly thinking of the "Assorted Constants" MPI_PROC_NULL through MPI_ROOT. There is probably prior discussion about these being compile-time vs link-time.

@jeffhammond
Copy link
Member Author

MPI_PROC_NULL is easy to make a compile-time constant because it's an int and we know that valid ranks are never negative, so we just pick our favorite negative number. Same for MPI_ROOT, I guess.

@jeffhammond
Copy link
Member Author

jeffhammond commented Dec 13, 2022

The only interesting one is MPI_BSEND_OVERHEAD because we have to get all implementations to agree on an upper-bound, but it's likely a small enough number that nobody cares. Also, depending on how we resolve mpi-forum#586, we might not need this one anymore.

MPI_BSEND_OVERHEAD is not required to be compile-time constant, so it can remain implementation specific. I don't know why I wrote this. I do not think it is correct.

@jeffhammond
Copy link
Member Author

Basically, for anything that has to be an integer and has no risk of conflicting with anything, I think we should just specify the constant.

@hzhou
Copy link

hzhou commented Feb 6, 2023

Of the following, only MPI_BSEND_OVERHEAD is nontrivial, because it relates to implementation details.

MPI_PROC_NULL
MPI_ANY_SOURCE
MPI_ANY_TAG
MPI_UNDEFINED
MPI_BSEND_OVERHEAD
MPI_KEYVAL_INVALID
MPI_LOCK_EXCLUSIVE
MPI_LOCK_SHARED
MPI_ROOT

How about making all these link-time constants? I think MPICH will be obligated to keep MPICH ABI for the near future -- likely > 10 years. Thus we cannot change the internal values for these constants. If these are defined as link time constants, we can easily maintain an abi-layer for these symbols.

If they are compile-time constants, we'll have to maintain a messy translation table.

@jeffhammond
Copy link
Member Author

jeffhammond commented Feb 7, 2023

Yeah, I favor making anything that is not already required to be a compile-time constant to be a link-time constant. That makes ABI compatibility layers easier. In Mukautuva, I just make a copy of every magic value in the implementation-specific SO and dlsym it at runtime. (I no longer do this.)

@qkoziol
Copy link

qkoziol commented Feb 7, 2023

Same

@qkoziol
Copy link

qkoziol commented Feb 7, 2023

@hzhou - Could you add them to the ABI header in the repo? (Whatever makes sense to you, then we can iterate)

jeffhammond added a commit to jeffhammond/mukautuva that referenced this issue Feb 7, 2023
@jeffhammond
Copy link
Member Author

I think we have a problem here, which requires us to make all the integer constants compile-time constants, or make a breaking change in the spec.

Recall:

All named constants, with the exceptions noted below for Fortran, can be used in initialization expressions or assignments, but not necessarily in array declarations or as labels in C switch or Fortran select/case statements. This implies named constants to be link-time but not necessarily compile-time constants. The named constants listed below are required to be compile-time constants in both C and Fortran. These constants do not change values during execution. Opaque objects accessed by constant handles are defined and do not change value between MPI initialization (MPI_INIT) and MPI completion (MPI_FINALIZE). The handles themselves are constants and can be also used in initialization expressions or assignments.

While MPI_DATATYPE_NULL can be done with link-time resolution using the same method of Open-MPI because it's a pointer, I don't think we can do this for the integer handles.

Consider this code:

#include <mpi.h>

static int s = MPI_ANY_TAG;
const int c = MPI_ANY_TAG;

int main(void)
{
  int m = MPI_ANY_TAG;
  (void)m;
  (void)s;
  (void)c;
  return MPI_ANY_TAG;
}

I believe this is compliant per §2.5.4, but it does not compile with Mukautuva because I am resolving the value of MPI_ANY_TAG during MPI initialization.

I don't see any way to make this work without defining the value in the header. I can't do the following , which would allow me to resolve the value at runtime.

extern void * MUK_MPI_ANY_TAG;
#define MPI_ANY_TAG (*(int*)MUK_MPI_ANY_TAG);

Am I missing something?

jeffhammond added a commit to jeffhammond/mukautuva that referenced this issue Feb 8, 2023
@hzhou
Copy link

hzhou commented Feb 8, 2023

Well, so be it :)

@jeffhammond Could you push a consolidated mpi_abi.h to https://github.com/mpiwg-abi/compatibility-layer, and I'll start working against it. While I understand your need to use separate headers in mukautuva, it is easier for down-stream consumption if it is a single mpi_abi.h. Maybe we should rename compatibility-layer to mpi_abi?

@jeffhammond
Copy link
Member Author

You can do whatever you want with naming things. The current Mukautuva development model is furious hacking so I am not using anything external, but I will eventually calm down and sync up with canonical ABI header repo.

Currently, you are using intptr_t for handles, whereas I am using incomplete struct pointers. If I change that, it will likely break things for you.

@jeffhammond
Copy link
Member Author

By the way, https://github.com/jeffhammond/mukautuva/blob/main/testconstants.c tests for the existence of nearly all of the compile-time integer named constants. I am now able to pass this test with Mukautuva. Not all of those constants are correctly translated to the implementation value, but many are.

In cases where MPICH and Open-MPI agree, I choose the same value. Comparison results, thread levels and MPI_UNDEFINED are such cases.

@hzhou
Copy link

hzhou commented Feb 8, 2023

Currently, you are using intptr_t for handles, whereas I am using incomplete struct pointers. If I change that, it will likely break things for you.

I believe we have agreed to use pointers. It'll be a quick adaption for me, but it is best to pin down a version to continue work on.

The mpi_abi.h in current https://github.com/mpiwg-abi/compatibility-layer is a quick hack during 2-day hackathon. Since you have worked out a much more complete version, I have no incentive to commit to the same labor that you already did. Rather, I would like to work from a common one with you to better coordinate effort.

In cases where MPICH and Open-MPI agree, I choose the same value. Comparison results, thread levels and MPI_UNDEFINED are such cases.

In either case, at least for now, we are adding runtime check/conversion since we can't rely on the assumption that any constants will match. This gives you total freedom in deciding these values. I would rather see the new ABI make clean sense, rather than to inherit artifacts from both OpenMPI and MPICH.

@hzhou
Copy link

hzhou commented Feb 8, 2023

We want this code to work, right? --

#include <mpi.h>

MPI_Comm comm = MPI_COMM_WORLD;

int main(void)
{
    MPI_Init(NULL, NULL);
    MPI_Barrier(comm);
    MPI_Finalize();
    return 0;
}

I don't think MPI_COMM_WORLD can be an external symbol because then its value won't be available for compiling. What OpenMPI did is define MPI_COMM_WORLD as a macro that refers to the address to an external symbol -- ompi_mpi_comm_world. So to make link-time constants work, we need to standardize secondary symbols, e.g. abi_mpi_comm_world, and then force implementations to adopt this abi. For OpenMPI, this means renaming their internal symbols such as ompi_mpi_comm_world. For MPICH, this means to force redefine its handle as pointers since there is no way we can have a link-time address to be a given value such as 0x4c000001. This will be impossible for MPICH in the near future.

I think that leaves the only option to define all these constants to be compile-time constants and have shim layer to translate the constants at runtime.

EDIT: it is possible for MPICH to work with link-time constants. MPICH need create dummy abi_mpi_comm_world and use the shim layer to do the same translation.

EDIT: There is a consideration for the shim translation. For compile-time constants, assuming the constants are in a lower range, it can be easily optimized into a simple range check to distinguish with builtin constants and dynamic values, and a table lookup for translating the builtin constants. If it is link time constants, then there is no optimizations. We are looking at a huge if-else if chain in the case of datatypes.

@jeffhammond
Copy link
Member Author

Yes, that code needs to work. Lots of code, including the MPICH test suite and the OSU MPI benchmarks, initialize to predefined handles before init.

I spent the day implementing this in MUK. I used a simplified version of the OMPI method. I think I did it without an if-else table for datatypes but I need to check it tomorrow.

I've made all the constants symbols when using MPICH by doing int IMPL_THREAD_SINGLE = MPI_THREAD_SINGLE etc for all the constants. The integer constants require translation tables.

Because MUK handles are (internally) pointers to pointers, I don't need if-else tables, or at least I didn't before the last redesign.

@qkoziol
Copy link

qkoziol commented Feb 16, 2023

I won’t steal Hui’s thunder by giving away the details here, but point out that he has a reasonable solution.

@hzhou - can you fill in the details about what we talked about this morning? :-)

@hzhou
Copy link

hzhou commented Feb 16, 2023

Sure. Give me a day or two to get pmodels/mpich#6390 in a working shape, then we can discuss by looking at the technical details.

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

No branches or pull requests

6 participants