Skip to content

Conversation

@kawashima-fj
Copy link
Member

@ggouaillardet Could you review?

Corresponding master PR: #3401

(cherry picked from commit 0fcd964)

Fortran constants `MPI_ARGV_NULL` and `MPI_ARGVS_NULL` are defined
in MPI-3.1 p.680 as below.

> `MPI_ARGVS_NULL`
>   2-dim. array of `CHARACTER*(*)`
> `MPI_ARGV_NULL`
>   array of `CHARACTER*(*)`

`MPI_ARGV_NULL` and `MPI_ARGVS_NULL` are used as an argument of
`MPI_COMM_SPAWN` and `MPI_COMM_SPAWN_MULTIPLE` respectively and
their argument `argv` and `array_of_argv` are defined as below
for `USE mpi_f08` binding in MPI-3.1.

```
CHARACTER(LEN=*), INTENT(IN) :: argv(*)
CHARACTER(LEN=*), INTENT(IN) :: array_of_argv(count, *)
```

Defining them as `INTEGER` in `mpi_f08` module will cause
a compilation error of user programs like
"There is no specific subroutine for the generic 'mpi_comm_spawn'".

Signed-off-by: KAWASHIMA Takahiro <t-kawashima@jp.fujitsu.com>

(cherry picked from commit open-mpi/ompi@0fcd964)
Copy link
Contributor

@ggouaillardet ggouaillardet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am fine with this.
strictly speaking, that might be an ABI change, but since we fix something that never worked before, i guess this is acceptable before v3.x
@jsquyres any thoughts ?

@hppritcha
Copy link
Member

I think given the nature of the bug that this PR fixes, it's okay even if it's technically an ABI change.

@jsquyres
Copy link
Member

I agree: it couldn't have worked before, so the fact that we're breaking ABI isn't too important here.

@jsquyres
Copy link
Member

@hppritcha Good to go.

@bwbarrett @hppritcha Merging this fix into v2.0.x/v2.x means you also need to merge #3533 into v3.0.x. It's a definite bug, and the risk/impact is low.

@hppritcha hppritcha merged commit 58f3524 into open-mpi:v2.0.x May 18, 2017
@kawashima-fj kawashima-fj deleted the pr/v2.0.x/fortran-argv-null branch November 24, 2017 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants