Skip to content
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

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

Closed
ceblanton opened this issue Jun 9, 2022 · 7 comments
Closed

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

ceblanton opened this issue Jun 9, 2022 · 7 comments

Comments

@ceblanton
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link

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
Copy link

bensonr commented Jun 9, 2022

@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
Copy link

bensonr commented Jun 9, 2022

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
Copy link
Contributor Author

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 pushed a commit to ceblanton/mkmf that referenced this issue Jun 23, 2022
- rename ncrc-intel.mk to ncrc-intel-classic.mk for clarity, and create new ncrc-intel-oneapi.mk
- For both, change -xsse2 to -march=core-avx2. This changes the ISA from xsse2 to avx2, and provides generic hardware support. It's believed the avx2 ISA can now support run-to-run reproducibility.
- For intel-classic, add -qno-opt-dynamic-align, needed for run-to-run reproducibility when using vector ISAs.
- For both, add -qopt-report-phase=vec and -qopt-report=2 to CFLAGS and FLAGS when VERBOSE is used
- For intel-oneapi, remove -sox from FFLAGS as ifx does not support it
ceblanton pushed a commit to ceblanton/mkmf that referenced this issue Jun 23, 2022
- rename ncrc-intel.mk to ncrc-intel-classic.mk for clarity, and create new ncrc-intel-oneapi.mk
- For both, change -xsse2 to -march=core-avx2. This changes the ISA from xsse2 to avx2, and provides generic hardware support. It's believed the avx2 ISA can now support run-to-run reproducibility.
- For intel-classic, add -qno-opt-dynamic-align, needed for run-to-run reproducibility when using vector ISAs.
- For both, add -qopt-report-phase=vec and -qopt-report=2 to CFLAGS and FLAGS when VERBOSE is used
- For intel-oneapi, remove -sox from FFLAGS as ifx does not support it
ceblanton pushed a commit to ceblanton/mkmf that referenced this issue Jun 23, 2022
ceblanton added a commit to ceblanton/mkmf that referenced this issue Jun 27, 2022
ceblanton added a commit to ceblanton/mkmf that referenced this issue Jun 27, 2022
ceblanton added a commit to ceblanton/mkmf that referenced this issue Jun 27, 2022
as C5's environment contains its own gettid Linux system call
ceblanton pushed a commit to ceblanton/mkmf that referenced this issue Jul 7, 2022
C5 will use different Intel templates, but the
C3/C4 Intel template should remain the same to avoid answer changes.
ceblanton pushed a commit to ceblanton/mkmf that referenced this issue Jul 7, 2022
… classic, and "production" or ifort/ifx

- For all 3, change -xsse2 to -march=core-avx2. This changes the ISA from xsse2 to avx2, and provides generic hardware support. It's believed the avx2 ISA can now support run-to-run reproducibility.
- For all 3, add -qopt-report-phase=vec and -qopt-report=2 to CFLAGS and FLAGS when VERBOSE is used
- For intel-classic and intel, add -qno-opt-dynamic-align, needed for run-to-run reproducibility when using vector ISAs.
- Also for intel-classic and intel, remove -ftrapuv from CFLAGS_DEBUG as icx (clang) does not support it
- For intel-oneapi, remove -sox from FFLAGS as ifx does not support it
ceblanton pushed a commit to ceblanton/mkmf that referenced this issue Jul 7, 2022
ceblanton added a commit to ceblanton/mkmf that referenced this issue Jul 7, 2022
ISA to FFLAGS as it is not a supported icx option
ceblanton added a commit to ceblanton/mkmf that referenced this issue Jul 7, 2022
as C5's environment contains its own gettid Linux system call
available in the GNU standard library. The intel-classic
environment does not have the gettid system function defined.

More details: NOAA-GFDL/FMS#276

The error one sees is:
src/FMS/affinity/affinity.c:55:14: error: static declaration of 'gettid' follows non-static declaration
static pid_t gettid(void)
             ^
/usr/include/bits/unistd_ext.h:34:16: note: previous declaration is here
extern __pid_t gettid (void) __THROW;
               ^
1 error generated.
ceblanton added a commit to ceblanton/mkmf that referenced this issue Jul 12, 2022
ceblanton added a commit to ceblanton/mkmf that referenced this issue Aug 3, 2022
based on the number of cores available.

This setting is undesired and potentially dangerous for batch jobs,
as it assumes the job can use all cores available. Instead, for batch
jobs the paralellism should be the number of cores allocated to the job.
(Updates to FRE do this.) For interactive jobs, users can set
MAKEFLAGS themselves (e.g. in their shell init files) or as
the make -j option.
ceblanton added a commit to ceblanton/mkmf that referenced this issue Sep 1, 2022
1. GNU Compiler Collection, not GNU
2. nvhpc instead of pgi
ceblanton added a commit to ceblanton/mkmf that referenced this issue Sep 1, 2022
…and remove

unsupported "-h nosecond_underscore" option
@ceblanton
Copy link
Contributor Author

done, in #44

ceblanton pushed a commit to ceblanton/mkmf that referenced this issue Oct 28, 2022
C5's environment contains its own gettid Linux system call
available in the GNU standard library.

More details: NOAA-GFDL/FMS#276

The error one sees is:
src/FMS/affinity/affinity.c:55:14: error: static declaration of 'gettid' follows non-static declaration
static pid_t gettid(void)
             ^
             /usr/include/bits/unistd_ext.h:34:16: note: previous declaration is here
             extern __pid_t gettid (void) __THROW;
                            ^
                            1 error generated.
ceblanton added a commit to ceblanton/mkmf that referenced this issue Nov 3, 2022
C5's environment contains its own gettid Linux system call
available in the GNU standard library

More details: NOAA-GFDL/FMS#276
ceblanton added a commit that referenced this issue Nov 3, 2022
 #42 For gaea C5 gcc make template, set HAVE_GETTID macro
ceblanton added a commit to ceblanton/mkmf that referenced this issue Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants