NOAA-GFDL / mkmf

Other
10 stars 51 forks source link

Update for ncrc-intel.mk needed for C5 #42

Closed ceblanton closed 2 years ago

ceblanton commented 2 years ago

From @nikizadehgfdl, we are planning to have a single Intel compiler template that works on both Intel and non-Intel hardware.

In the past, we had switched to the xsse2 instruction set to support run-to-run reproduciblity, and it is believed that this is no longer needed for recent Intel compilers.

The update needed is the ISA. Existing:

ISA = -xsse2

Proposed:

ISA = -march=core-avx2 -qopt-report-phase=vec -qopt-report=2 -qno-opt-dynamic-align

Note: this is two changes: 1) to avx2, and 2) generic hardware support. The first change only would be:

ISA = -xCORE-AVX2 -qopt-report-phase=vec -qopt-report=2 -qno-opt-dynamic-align

ceblanton commented 2 years ago

More details from @nikizadehgfdl:

-march=core-avx2 is the needed update

-qno-opt-dynamic-align might not be needed or wanted. It appears to disable a potential optimization (Intel docs)

Do we want this option, and if so, should it be removed from the ISA definition and added elsewhere? (As it is not related to instruction sets)

-qopt-report-phase=vec and -qopt-report=2 increase the verbosity only (do not affect results).

Do we want this additional verbosity all the time? There are some additional verbose flags that are not used normally in the templates:

https://github.com/NOAA-GFDL/mkmf/blob/master/templates/ncrc-intel.mk#L169-L173

If users want the additional verbosity all the time, these can be added to the CFLAGS and FFLAGS but probably should be removed from the ISA definition as they are not instruction set-related.

If users want the additional verbosity rarely, they can be added to CFLAGS_VERBOSE and FLAGS_VERBOSE.

ceblanton commented 2 years ago

Hi @bensonr, could you provide some feedback on the proposed changes to ncrc-intel.mk by @nikizadehgfdl ?

Specifically, whether -qno-opt-dynamic-align is needed and if so, should it be moved from ISA for clarity?

And then for the two others that increase verbosity, should they be used all the time, or only if the VERBOSE macro is set? (They shouldn't be part of the ISA macro for clarity, I think)

nikizadehgfdl commented 2 years ago

I just started compiling on t5 login node. With intel-oneapi/2022.0.2 I get the following warnings

ifx: command line warning #10148: option '-sox' not supported
ifx: command line warning #10148: option '-qno-opt-dynamic-align' not supported
ifx: command line warning #10121: overriding '-march=core-avx2' with '-march=core-avx2'

So, -qno-opt-dynamic-align is not even supported and there is no point putting it in the c5 template, neither is -sox. And -march=core-avx2 seems to be the default.

bensonr commented 2 years ago

@nikizadehgfdl - this template is for intel classic ifort and not for use with ifx. The -qno-opt-dynamic-align is necessary for run-to-run reproducibility when using vector ISAs. Without this option, the runtime environment is free to change data alignment, which changes what data can be pipelined.

bensonr commented 2 years ago

I should also add that any C/C++ options will probably need to change with icx/icpcx due to it being a Clang-based front-end. I think that will make it hard to maintain a single intel.mk file in favor of a classic version and a new oneapi.mk version.

ceblanton commented 2 years ago

Since -qno-opt-dynamic-align is required for intel-classic and not supported in intel-oneapi, I agree with @bensonr's suggestion to maintain a classic version and a new intel-oneapi.mk version. We could support both in Bronx-20.

What about the options that increase verbosity (-qopt-report-phase=vec -qopt-report=2)? I suggest they be used only when the VERBOSE macro is set.

ceblanton commented 2 years ago

done, in #44