Redundant use statements due to #include lines

[mark-petersen]

Some Fortran files in core_ocean include *.inc files. These files have many instances of redundant use statements, and the result is that a file like src/core_ocean/driver/mpas_ocn_core_interface.f90 has many redundant use statements (77 in one example). Intel staff member Martyn Corden said that modules only need one use statement (before the contains statement) and multiple use statements can degrade performance. This mirrors an issue by Noel Keen on the ACME repo:
E3SM-Project/E3SM#1176

[mark-petersen]

In a nutshell, we just have lines like use mpas_dmpar in every single subroutine imported by these lines:

wf-fe1.lanl.gov> pwd
/turquoise/usr/projects/climate/mpeterse/repos/MPAS/temp/src/core_ocean/driver
wf-fe1.lanl.gov> tail mpas_ocn_core_interface.F
#include "../inc/define_packages.inc"
#include "../inc/structs_and_variables.inc"
#include "../inc/namelist_call.inc"
#include "../inc/namelist_defines.inc"
end module ocn_core_interface

wf-fe1.lanl.gov> grep 'use mpas_dmpar' mpas_ocn_core_interface.F | wc -l
1
wf-fe1.lanl.gov> grep 'use mpas_dmpar' mpas_ocn_core_interface.f90 | wc -l
77

In the original *.F file you only see the include statements. You need to compile with make ... GEN_F90=true to see the multiple use statements in the *.f90 files.

[mark-petersen]

I'm not sure if this is a framework issue, or core specific. The offending files are in the src/core_ocean/driver directory, but the *.inc files appear to be generated from framework. I'm adding both tags.

[mark-petersen]

This is text by Noel Keen, an ACME performance team member:
On Cori, I ran into a build issue with Intel v17 (both versions on Cori) in the ocean. (See error message pasted in below). Looking closer in the code, there does not seem to be a problem at all! So assuming it was a compiler oddity, I found a simple work-around and continued working. The compiler is complaining that it thinks the integer variable ierr is brought in from a module and then also declared locally. But it is not brought in from a module. I simply renamed the local variable from ierr to ierrz and it compiled/worked. To fix this for the next time, I modified components/mpas-o/model/src/tools/registry/gen_inc.c to insert the code with this renamed local variable (ierrz instead of ierr). I don't see this issue with Intel v16. I'm including the source file as it's one of those auto-generated ones and it's good to see exactly what the compiler is trying to compile.
mpas_ocn_core_interface.f90.txt

I also brought this up to Intel staff member Martyn Corden. I was granted permission to give this one source file to him. He replied:

...I don’t see the problem in small test cases, unless a USEd
module really does have a public declaration of IERR.
 
It struck me that you were re-USE-ing the same modules over and over
from different subroutines, even though they were used at the head of
the module ocn_core_interface. This isn’t necessary and can be
counterproductive. For example, mpas_derived_types was USEd >150
times.

I suggest an experiment, where you comment out (or remove) every
instance of the following statements that appears after the contains
statement:
 
   use mpas_derived_types
   use mpas_pool_routines
   use mpas_dmpar
   use mpas_io_units
 
and see 1) if the problem persists, and 2) whether the compilation
goes any faster.  (This used to be a problem for compilation time in
older compilers, but that may have been fixed by now. It’s also
possible that the fix might be related to the problem in 17.0).

ftn  -convert big_endian -assume byterecl -ftz -traceback -assume realloc_lhs -fp-model source -xMIC-AVX512 -diag-disable 10121 -O2 -qno-opt-dynamic-align -DMPAS_NO_LOG_REDIRECT -DMPAS_NO_ESMF_INIT -DMPAS_ESM_SHR_CONST -DMPAS_PERF_MOD_TIMERS -c mpas_ocn_core_interface.f90
...
mpas_ocn_core_interface.f90(85608): error #6405: The same named entity from different modules and/or program units cannot be refe
renced.   [IERR]
         read(unitNumber, AM_pointwiseStats, iostat=ierr)
----------------------------------------------------^
compilation aborted for mpas_ocn_core_interface.f90 (code 1)
Makefile:18: recipe for target 'mpas_ocn_core_interface.o' failed
make[3]: *** [mpas_ocn_core_interface.o] Error 1
make[3]: Leaving directory '/global/cscratch1/sd/ndk/acme_scratch/SMS.T62_oEC60to30.GMPAS.cori-knl_intel.m21n041/bld/ocn/source/c
ore_ocean/driver'
Makefile:8: recipe for target 'all' failed
make[2]: *** [all] Error 2
make[2]: Leaving directory '/global/cscratch1/sd/ndk/acme_scratch/SMS.T62_oEC60to30.GMPAS.cori-knl_intel.m21n041/bld/ocn/source/c
ore_ocean'
Makefile:39: recipe for target 'dycore' failed

[mark-petersen]

Again from Noel Keen
OK, the change Martyn suggested does appear to get us past this issue. Can we make it?

That is, in components/mpas-o/model/src/tools/registry/gen_inc.c

<     //fortprintf(fd, "      use mpas_derived_types\n");
<     //fortprintf(fd, "      use mpas_dmpar\n");
<     //fortprintf(fd, "      use mpas_pool_routines\n");
<     //fortprintf(fd, "      use mpas_io_units\n");
---
>     fortprintf(fd, "      use mpas_derived_types\n");
>     fortprintf(fd, "      use mpas_dmpar\n");
>     fortprintf(fd, "      use mpas_pool_routines\n");
>     fortprintf(fd, "      use mpas_io_units\n");

[mark-petersen]

This issue was solved by #1208