-
Notifications
You must be signed in to change notification settings - Fork 843
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
configury: disable f08 fortran bindings if the compiler does #315
Conversation
Refer to this link for build results (access rights to CI server needed): |
I wouldn't call this "check for TS29113", because TS29113 contains a bunch of stuff. How about renaming to CHECK_C_FUNCLOC? |
Also: the results of whether the compiler has C_FUNLOC or not should be added to ompi_info output. |
b916de9
to
960bf76
Compare
@jsquyres i made the requested changes, rebased and updated the PR. as a side note, i kept the same logic used for other checks :
That means that for example, if f08 bindings are not requested,
even if the compiler is TS29113 subclause 8.1 compliant. |
Refer to this link for build results (access rights to CI server needed): |
@@ -426,6 +428,16 @@ AC_DEFUN([OMPI_SETUP_MPI_FORTRAN],[ | |||
[OMPI_FORTRAN_HAVE_OPTIONAL_ARGS=0 | |||
OMPI_BUILD_FORTRAN_USEMPIF08_BINDINGS=0])]) | |||
|
|||
OMPI_FORTRAN_HAVE_C_FUNLOC=0 | |||
AS_IF([test $OMPI_WANT_FORTRAN_USEMPIF08_BINDINGS -eq 1 -a \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm quite sure that we're not consistent about this, but @bertwesarg is right: test -a is not portable.
Instead, use AS_IF([test ..blah... && test ...blah... ], ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I take this back. The rest of ompi_setup_mpi_fortran.m4 uses -a and -o. So go ahead and leave it here.
But let's do another blanket-change commit after this one that changes them all to && and ||.
not support c_funloc with TS 29113 subclause 8.1 aka removed restrictions on ISO_C_BINDING module procedures.
960bf76
to
27aec2e
Compare
@jsquyres i added the clause in ompi_info once this is reviewed, i ll make a PR for v1.8 and rework the config files to use the portable syntax as advised by @bertwesarg |
Refer to this link for build results (access rights to CI server needed): Build Log
Test FAILed. |
@miked-mellanox This looks like a false jenkins failure. Do you know why the thread tests were run? The word "thread" is not in the PR title. |
@jsquyres - yep, since it got fixed and merged - i enabled it by default on, was it too soon? |
@miked-mellanox I don't know -- this was clearly a false failure, though. So I'm not sure what happened here. |
it fails for me with tcp on clean repo on some benchmarks. currently i disabled tcp in jenkins (canbe re-enabled with "known_issues" |
A first cut at a possible solution for the missing requests
Fix support for "-x" cmd line option Also extend it to support wildcards - e.g., "-x foo*" to forward all envars starting with "foo" Signed-off-by: Ralph Castain <rhc@pmix.org>
not implement TS 29113
Per http://www.open-mpi.org/community/lists/users/2014/12/25963.php sunstudio compiler 12.4 fail compiling Open MPI (f08 bindings).
The root cause is these compilers do not implement TS 29113 subclause 8.1 :
Removed restrictions on ISO_C_BINDING module procedures
This PR disables f08 fortran bindings support if TS 29113 8.1 is not implemented.
@jsquyres could you please review this ?