-
Notifications
You must be signed in to change notification settings - Fork 901
Fix Fortran preprocessor issue with CPPFLAGS #9899
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
Conversation
bot:aws:retest |
Example output with Nvidia/PGI compiler v22.1-0:
Example with gcc10:
This will need to be cherry-picked to the |
Does this skip the check for the |
It looks like if you pass |
0a3ea64
to
4f3729c
Compare
I just pushed a fix for the
|
Note that once this fix gets into |
This looks tangentially related to #9890 in that it's an issue about the C and Fortran compiler flags. |
@jjhursey This feels very much like #7253 (issue) and #7265 (PR to fix it). E.g., in the commit message for ab398f4, I stated:
Did we miss adding something like https://github.com/open-mpi/ompi/blob/master/ompi/mpi/fortran/use-mpi-f08/Makefile.am#L29-L34 somewhere? Can you cite the specific problem that you're solving -- e.g., are you seeing |
The error is:
A bit more detail:
Is the solution roughly this to clear the |
I tried the alternative approach and it worked fine. diff --git a/ompi/mpiext/ftmpi/use-mpi-f08/Makefile.am b/ompi/mpiext/ftmpi/use-mpi-f08/Makefile.am
index d06c0aa6..85d1b778 100644
--- a/ompi/mpiext/ftmpi/use-mpi-f08/Makefile.am
+++ b/ompi/mpiext/ftmpi/use-mpi-f08/Makefile.am
@@ -13,6 +13,14 @@
# $HEADER$
#
+# Note that Automake's Fortran-buidling rules uses CPPFLAGS and
+# AM_CPPFLAGS. This can cause weirdness (e.g.,
+# https://github.com/open-mpi/ompi/issues/7253 and
+# https://github.com/open-mpi/ompi/issues/9716). Let's just zero
+# those out and rely on AM_FCFLAGS.
+CPPFLAGS =
+AM_CPPFLAGS =
+
# This file builds the use_mpi_f08-based bindings for MPI extensions. It
# is optional in MPI extensions. I went ahead and added the change above to this PR as a second commit. That'll make it easier to pick the one the community wants. I'm happy with either or both. I'm just looking for some guidance from the community on their preference. Let me know and I'll update the PR. |
I'd prefer the second approach (mucking with CPPFLAGS) for two reasons: 1) we already do it elsewhere and 2) I'm worried about the corner cases that led to -iquote in the first place biting us in the butt again. |
Sure. I'll make that adjustment and update this PR in a bit. |
* Some C and Fortran compilers use different preprocessors. If one preprocessor accepts `-iquote` and the other does not then a compiler error will occur when Open MPI tries to use it. - Nvidia/PGI v22.1-0 is one such. The C compiler supports `-iquote` while the Fortran compiler does not. * Similar to PR open-mpi#7265 we need to clear the `CPPFLAGS` and `AM_CPPFLAGS` Signed-off-by: Joshua Hursey <[email protected]>
0a6b56a
to
bbe5788
Compare
@jsquyres @bwbarrett I made those changes. This is ready for re-review. Thanks! |
accepts
-iquote
and the other does not then a compiler error will occurwhen Open MPI tries to use it.
-iquote
while the Fortran compiler does not.
CPPFLAGS
andAM_CPPFLAGS
Signed-off-by: Joshua Hursey [email protected]