Skip to content

Conversation

@hjelmn
Copy link
Member

@hjelmn hjelmn commented Aug 15, 2017

This commit adds a new configure option: --with-wrapper-cc. This
option can be used to set which C compiler is invoked by mpicc. The
feature is necessary when building for compiler suites that do not
support C11 (Ex: icc). A version of Open MPI that is set up for these
compilers can be built with:

./configure --with-wrapper-cc=icc CC=gcc FC=ifort CXX=icpc

Signed-off-by: Nathan Hjelm [email protected]

This commit adds a new configure option: --with-wrapper-cc. This
option can be used to set which C compiler is invoked by mpicc. This
feature may necessary when building for compiler suites that do not
support C11 (Ex: icc) if Open MPI chooses to require a C11 compiler. A
version of Open MPI that is set up for these compilers can be built
with:

./configure --with-wrapper-cc=icc CC=gcc FC=ifort CXX=icpc

Signed-off-by: Nathan Hjelm <[email protected]>
@hjelmn
Copy link
Member Author

hjelmn commented Aug 15, 2017

@jsquyres Thoughts? I think this is a useful configure option even if we do not chose to require C11. Should I add similar options for mpicxx and mpifort for completeness?

@rhc54
Copy link
Contributor

rhc54 commented Aug 15, 2017

Urgggg....ick. I'm not much of a company man, as you know, but I think I can hear the howl from HQ from here.

@hjelmn
Copy link
Member Author

hjelmn commented Aug 15, 2017

@rhc54 Yeah, needing this would be unfortunate. Intel really dropped the ball on C11. Both clang and gcc are years ahead of them.

@bwbarrett
Copy link
Member

@rhc54, I think your comments were more about Nathan's comments in #3879 than in this change, right?

@rhc54
Copy link
Contributor

rhc54 commented Aug 15, 2017

@bwbarrett I was actually commenting about both, I guess - it was this configure option thing that caught my immediate eye as it singles out the compilers from one vendor. Won't sit well, and I concur with your statement over there about being careful about what is needed vs what it might cost us.

@hjelmn
Copy link
Member Author

hjelmn commented Aug 15, 2017

@rhc54 I can remove the reference to icc from the commit message. Intel is just the one that motivated this particular PR :).

@rhc54
Copy link
Contributor

rhc54 commented Aug 15, 2017

I don't think that really resolves the problem. As was said on the other issue, this just creates more ways things can go wrong, and actually advocates cross-compiler builds (which we generally have said is "bad").

@hjelmn
Copy link
Member Author

hjelmn commented Aug 15, 2017

This is generally how libraries are compiled. Usually they are built with gcc, clang, etc. The application itself is compiled with pgi, intel, gcc, etc. MPI is slightly different in that we provide wrappers for the users and we have fortran support. I don't think an Open MPI libmpi.so compiled with gcc and linked into an application that was built with icc is a novel or dangerous idea.

@hjelmn
Copy link
Member Author

hjelmn commented Aug 15, 2017

We probably should discuss this in a future call. Can't be next week but the week after should work fine.

@hppritcha
Copy link
Member

@hjelmn please rebase

@jsquyres
Copy link
Member

jsquyres commented Nov 7, 2018

@hjelmn Do you still care about this PR? If so, it needs to be rebased.

@hjelmn
Copy link
Member Author

hjelmn commented Nov 7, 2018

Yes we do. If we tighten compiler requirements we will need this. Will rebase it today.

Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

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

Needs to be rebased before this can continue.

@awlauria
Copy link
Contributor

Ping. Rebase is needed still if still desired.

@lanl-ompi
Copy link
Contributor

Can one of the admins verify this patch?

@jsquyres jsquyres marked this pull request as draft October 26, 2020 15:29
@jsquyres
Copy link
Member

This PR is pretty stale, but could be fairly easily fixed if the functionality is actually still desired. I moved it to "draft" state.

@rhc54
Copy link
Contributor

rhc54 commented Feb 1, 2023

@hjelmn You have several PRs that date back 3-6 years - would it make sense for you to triage them and close the ones not worth rebasing, fixing, resubmitting for review, and finally committing?

@hppritcha
Copy link
Member

I think this PR can be closed as never to merged. @jsquyres

@jsquyres
Copy link
Member

Yes, let's close. Can be re-opened / re-created if someone wants this functionality.

@jsquyres jsquyres closed this Oct 21, 2024
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.

7 participants