geoschem / gchp_legacy

Repository for GEOS-Chem High Performance: software that enables running GEOS-Chem on a cubed-sphere grid with MPI parallelization.
http://wiki.geos-chem.org/GEOS-Chem_HP
Other
7 stars 13 forks source link

[BUG/ISSUE] Suggested fix for C-preprocessor error when using ifort and gcc5+ #16

Closed JiaweiZhuang closed 4 years ago

JiaweiZhuang commented 5 years ago

(I am using my Intel student license on AWS; this allows me to skip #15 for now and test high-resolution multi-node runs as quickly as possible.)

Problem

I got this compile error when using ifort18 + gcc5.4.0 on Ubuntu 16.04:

<stdin>:2046:22: error: C++ style comments are not allowed in ISO C90
<stdin>:2046:22: error: (this will be reported only once per input file)
/home/ubuntu/tutorial/gchp_standard/CodeDir/GCHP/Shared/Config/ESMA_base.mk:367: recipe for target 'ESMFL_Mod.o' failed
gmake[8]: *** [ESMFL_Mod.o] Error 1
gmake[8]: *** Waiting for unfinished jobs....
<stdin>:4653:54: error: C++ style comments are not allowed in ISO C90
<stdin>:4653:54: error: (this will be reported only once per input file)
/home/ubuntu/tutorial/gchp_standard/CodeDir/GCHP/Shared/Config/ESMA_base.mk:367: recipe for target 'MAPL_IO.o' failed
gmake[8]: *** [MAPL_IO.o] Error 1
gmake[8]: Leaving directory '/home/ubuntu/tutorial/Code.GCHP/GCHP/Shared/MAPL_Base'
GNUmakefile:65: recipe for target 'install' failed
gmake[7]: *** [install] Error 2
gmake[7]: Leaving directory '/home/ubuntu/tutorial/Code.GCHP/GCHP/Shared/MAPL_Base'

Full log: compile_ifort_icc_bug.log

Suggested fix

fe5e863 fixes the same issue for gfortran, but the intel block remains unchanged:

https://github.com/geoschem/gchp/blob/7a4589c276876b6674800f4e4137b575e4def4f5/Shared/Config/ESMA_base.mk#L301-L313

I made the same change for intel, and then the compile error was gone:

 ifeq ("$(ESMF_COMPILER)","intel")
   FREAL8      = -r8
   FREE        =
-  CPPANSIX    = -ansi -DANSI_CPP
+  CPPANSIX    = -std=gnu11 -nostdinc -C

Note that it doesn't matter whether icc or gcc is set as $CC. The preprocessor is hard-coded as gcc:

https://github.com/geoschem/gchp/blob/7a4589c276876b6674800f4e4137b575e4def4f5/ESMF/build/common.mk#L611

After changing CPPANSIX, GCHP can compile correctly with either CC=icc or CC=gcc (both failed originally):

On Odyssey, the default gcc version is 4.8.5, so there is no such problem. I expect that gcc5+ will all have this issue.

Additional info

I found an old email from Daniel Rothenberg, which explained this issue well:

MAPL_Base was really tough to compile because of MAPL_IO.P90 and ESMFL_Mod.P90. The issue is the intermediate step of using cpp to pre-process them before Fortran compilation; a flag “-ansi” is hard-coded into this step in the Config/ESMA_base.mk makefile, which my local gcc (v6.3.1) really didn’t like. It kept failing to process the P90 files, throwing an error

C++ style comments are not allowed in ISO c90 but disabling these warnings didn’t work. The issue is that the P90 files contain Fortran string concatenation operators (“//“) which cpp can’t handle elegantly. Totally disabling comment removal with the “-C” flag worked, but produced license comments that ifort then choked on.

The only solution I came up with was to replace the pre-processing rule and instead force it to use Intel’s fpp, but for this to work correctly I had to hard-code it into the rule.

JiaweiZhuang commented 5 years ago

BTW I did a quick performance comparison of gfortran7.2.0 vs ifort18 on EC2 r5.2xlarge:

With gfortran, FV3 is slower by 70%, other components are as fast, and the entire model is slower by 10%. Pretty acceptable. Once gfortran is more robust (say, used in our official benchmark) I will switch back to it.

lizziel commented 4 years ago

I am closing this issue since the GNU makefiles will be retired in 13.0.0.