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

Score-P EB: Select the oneAPI compiler suite when enabled in the Intel compiler toolchain #3228

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

bertwesarg
Copy link

No description provided.

@bertwesarg
Copy link
Author

@geimer @cfeld @Flamefire please give this some eyes, thanks

Copy link
Contributor

@Flamefire Flamefire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When exactly does this need to be used? I.e. what is the effect of using "oneapi" over "intel"? If I understood you correctly the configure used by those projects will ignore $CC/$CXX and similar env variables, is this correct? What is used instead?

This would need to be clarified first especially for the hard failure for "to old" versions introduced here.

If I understand this correctly "intel-compilers" starting at some point include "oneAPI compilers" which differ in the naming (e.g. icx instead of icc). But they seemingly still include the old compilers (at least until 2024 releases), i.e. the module contains both. Is that correct?
If that is the case then we can use either one in this easyblock depending on the version, can't we?
Reason for the question is what we need to check here: Do we care about those new releases containing oneAPI compilers in general or only if oneAPI compilers were enabled to be used (by toolchain options)? I.e. what exactly is/is not supported by Score-P & Co?

I think we also need to revise the logic a bit: In the toolchain definition self.toolchain.options['oneapi'] defaults to None but if set takes precedence over "oneapi_c_cxx" & "oneapi_fortran". This should be mirrored here.

Maybe it is better to use is_oneapi_cc = self.toolchain.COMPILER_CC != IntelIccIfort.COMPILER_CC and similar for COMPILER_FC to detect if oneAPI is in use

Also the error for "mixed Intel oneAPI and classic Fortran family" might be removed:

  • Just enabling "oneapi" should be fine, especially "ifx and ifort are binary (.o/.obj) and module file (.mod) compatible."
  • It should at least take self.toolchain.options['oneapi'] into account (see above)

Comment on lines +90 to +91
# Since intel-compilers 2022.2.0 it uses the icx/icpx compilers, AfS projects support them
# since these version
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version is a detail which I don't think is relevant here. Also "AfS" is unknown at this point. Maybe:

Suggested change
# Since intel-compilers 2022.2.0 it uses the icx/icpx compilers, AfS projects support them
# since these version
# Intel oneAPI compilers (icx, icpx...) are supported since these versions:

# AfS packages do not yet support oneAPI compilers
raise EasyBuildError("Support to build with Intel oneAPI family started with " +
f"{self.name} {oneapi_since.get(self.name, '0')} but not {self.version}")
if not self.toolchain.options.get('oneapi_fortran', False):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fail in default scenarios: For "intel-compilers >= 2022.2.0" only oneapi_c_cxx is enabled by default, but not oneapi_fortran. I opened an issue asking for clarification/change

@bertwesarg bertwesarg marked this pull request as draft February 15, 2024 10:03
@Thyre
Copy link

Thyre commented Feb 15, 2024

When exactly does this need to be used? I.e. what is the effect of using "oneapi" over "intel"? If I understood you correctly the configure used by those projects will ignore $CC/$CXX and similar env variables, is this correct? What is used instead?

This would need to be clarified first especially for the hard failure for "to old" versions introduced here.

Score-P's build system is a bit complicated. To select a compiler during configure, we offer several platform files, which can be selected via --with-nocross-compiler-suite=[x] with a similar flag existing for cross-compiling. This is also the case for things like MPI and SHMEM.

Our intention is to have a combination of compilers we're certain are working well together. Theoretically, one can set ./configure CC=[my-c-compiler] [...], but this is not the intended way. For this, we offer --with-custom-compilers if a configuration is not supported by Score-P. Please correct me if I'm wrong here @bertwesarg.

Support for the LLVM-based oneAPI compilers was introduced in Score-P version 8.0. If one uses --with-nocross-compiler-suite=oneapi with an older Score-P version, the configure check likely aborts with an error message. Therefore, aborting in the EasyBlock seems reasonable to me. Theoretically, its the same for NVHPC and ROCm/AOMP amdclang, which are also supported since Score-P v8.0.

If I understand this correctly "intel-compilers" starting at some point include "oneAPI compilers" which differ in the naming (e.g. icx instead of icc). But they seemingly still include the old compilers (at least until 2024 releases), i.e. the module contains both. Is that correct? If that is the case then we can use either one in this easyblock depending on the version, can't we? Reason for the question is what we need to check here: Do we care about those new releases containing oneAPI compilers in general or only if oneAPI compilers were enabled to be used (by toolchain options)? I.e. what exactly is/is not supported by Score-P & Co?

You're right that "intel-compilers" started to include newer LLVM-based compilers at some point. With this, both icc/icpc/ifort and icx/icpx/ifx are available. Intel started to deprecate the older compilers in favor of the new ones back in 2023, with the removal of icc and icpc with version 2024.0. ifort will stick around for a few more releases, but will eventually also get removed.

In theory, Score-P can handle both compilers variants, but only one per installation.
Since Score-P will provide wrappers for the selected compilers for instrumentation, the EasyBlock should probably select the selected compilers by the toolchain options to decide if we want to use oneapi or intel. Mixing both is not intended and may lead to issues later. One reason why the legacy compilers were superior compared to the LLVM-based ones was, that the ifort compiler did support compiler instrumentation flags. Those were added to ifx in 2024.0.

Also the error for "mixed Intel oneAPI and classic Fortran family" might be removed:

* Just enabling "oneapi" should be fine, especially "ifx and ifort are binary (.o/.obj) and module file (.mod) compatible."

* It should at least take `self.toolchain.options['oneapi']` into account (see above)

As mentioned before, mixing the compilers is not intended us, though it may still work if F77, F90 and so on are set manually, as many configure checks are done on a language basis.

@Flamefire
Copy link
Contributor

@Thyre Thanks for the explanation

Score-P's build system is a bit complicated. To select a compiler during configure, we offer several platform files, which can be selected via --with-nocross-compiler-suite=[x] with a similar flag existing for cross-compiling. This is also the case for things like MPI and SHMEM.

As confirmed by Bert those "platform files" set/override the relevant CC/CXX/... variables. So that is basically the relevance of this flag in this context.

Our intention is to have a combination of compilers we're certain are working well together. Theoretically, one can set ./configure CC=[my-c-compiler] [...], but this is not the intended way. For this, we offer --with-custom-compilers if a configuration is not supported by Score-P. Please correct me if I'm wrong here @bertwesarg.

This sounds to me like what we should use. Especially as "ifort compiler did support compiler instrumentation flags. Those were added to ifx in 2024.0."

So IMO the behavior currently used by EasyBuild (use new C/C++ compilers but old Fortran compiler by default) seems to match the best support, doesn't it?

However if I understood correctly the configure will also set compiler flags based on that and hence might set unsupported flags. But when I look at e.g. build-config/common/platforms/compiler-frontend-oneapi it seems to mostly be -g -O2 or -g -O2 -fp-model=precise -diag-disable=10441 for oneapi & intel respectively. So I guess we could use the default EB flags too, couldn't we?

If one uses --with-nocross-compiler-suite=oneapi with an older Score-P version, the configure check likely aborts with an error message. Therefore, aborting in the EasyBlock seems reasonable to me.

Agreed. We just have to be sure to get the conditions correct. See my suggestion to check COMPILER_CC instead of the toolchain options as the former will match the actual compiler selection used in other places (e.g. to set $CC and such)
My idea was to switch back to intel instead of oneapi for those older versions given that the compilers are still there and could be used. Especially as EB automatically enables OneAPI compilers, so it's not always a user choice which would make the build fail here.
But this could too be confusing and avoiding the failure can be easily done by toolchainopts = {'oneapi': False} (once the conditions here are correct)

Mixing both is not intended and may lead to issues later. One reason why the legacy compilers were superior compared to the LLVM-based ones was, that the ifort compiler did support compiler instrumentation flags. Those were added to ifx in 2024.0.

See above: Wouldn't it be possible and beneficial to mix the new C/C++ with the old iffort for best feature support?

Also the error for "mixed Intel oneAPI and classic Fortran family" might be removed:

* Just enabling "oneapi" should be fine, especially "ifx and ifort are binary (.o/.obj) and module file (.mod) compatible."

* It should at least take `self.toolchain.options['oneapi']` into account (see above)

As mentioned before, mixing the compilers is not intended us, though it may still work if F77, F90 and so on are set manually, as many configure checks are done on a language basis.

But --with-nocross-compiler-suite=oneapi will select the new Fortran compiler, wouldn't it? So even though F77/F90 are set by EB (possibly to the old compilers) it wouldn't matter for the configure.
My point is that there are 3 toolchain options: oneapi_c_cxx & oneapi_fortran for per-language setting and oneapi for both, taking precedence if set. And EB by default (i.e. none of them set) will enable oneapi_c_cxx but not oneapi_fortran based on the intel-compilers module version. So we will be very often in the situation that would trigger the error (based on oneapi_c_cxx != oneapi_fortran)
My suggestion was that if any language uses the new compilers, configure should be instructed to use all new compilers instead of erroring.
Sidepoint being that the current condition is wrong and will trigger the error when it should not, i.e. when oneapi=True; oneapi_fortran=False because the precedence rule of oneapi isn't correctly handled here.

@cfeld
Copy link
Contributor

cfeld commented Feb 16, 2024

@Thyre Thanks for the explanation

Score-P's build system is a bit complicated. To select a compiler during configure, we offer several platform files, which can be selected via --with-nocross-compiler-suite=[x] with a similar flag existing for cross-compiling. This is also the case for things like MPI and SHMEM.

As confirmed by Bert those "platform files" set/override the relevant CC/CXX/... variables. So that is basically the relevance of this flag in this context.

Our intention is to have a combination of compilers we're certain are working well together. Theoretically, one can set ./configure CC=[my-c-compiler] [...], but this is not the intended way. For this, we offer --with-custom-compilers if a configuration is not supported by Score-P. Please correct me if I'm wrong here @bertwesarg.

This sounds to me like what we should use. Especially as "ifort compiler did support compiler instrumentation flags. Those were added to ifx in 2024.0."

So IMO the behavior currently used by EasyBuild (use new C/C++ compilers but old Fortran compiler by default) seems to match the best support, doesn't it?

This would provide the best compiler instrumentation support for oneAPI releases prior 2024.0. From 2024.0 on, using icx/icpx/ifx should be fine.

But Score-P's configure option --with-nocross-compiler-suite does support either intel, using the classic icc/icpc/ifort, or oneapi, using icx/icpx/ifx. There is no --with-nocross-compiler-suite option to select the oneAPI/classic mix icx/icpx/ifort.

To configure a oneAPI/classic mix, one can either use --with-custom-compiler-suite or ./configure CC=... MPICC=... SHMEMCC=... ..., see Score-P's INSTALL file for a complete list of environment variables.

Score-P's configure assumes that MPI (build-mpi) and SHMEM (build-shmem) compiler wrappers use compilers compatible to CC/CXX/F77/FC (build-backend). For MPICH-based MPIs Score-P takes care of this by setting -cc/-cxx/-fc. For Open MPI we assume that it was configured correctly (we don't use OMPI_CC etc. (yet)). With EasyBuild using a oneAPI/classic mix by default, and Score-P overriding this default to either classic or oneAPI for build-backend, we end up with different compilers for build-backend and build-mpi|shmem. Is this a problem? I don't know.

However if I understood correctly the configure will also set compiler flags based on that and hence might set unsupported flags. But when I look at e.g. build-config/common/platforms/compiler-frontend-oneapi it seems to mostly be -g -O2 or -g -O2 -fp-model=precise -diag-disable=10441 for oneapi & intel respectively. So I guess we could use the default EB flags too, couldn't we?

On a no-cross-compile machine, configure uses all variables from compiler-nocross-<vendor>, compiler-mpi-<mpi>, compiler-shmem-<shmem>. The top-level configure puts them explicitly on the configure lines of the build-backend|mpi|shmem configures, thus overriding EasyBuild variables of the same name.

With moderate changes to Score-P/Scalasca/OTF2/CUBE* one could prepend to *FLAGS, but I don't see the benefit. Please elaborate if you disagree.

If one uses --with-nocross-compiler-suite=oneapi with an older Score-P version, the configure check likely aborts with an error message. Therefore, aborting in the EasyBlock seems reasonable to me.

Agreed. We just have to be sure to get the conditions correct. See my suggestion to check COMPILER_CC instead of the toolchain options as the former will match the actual compiler selection used in other places (e.g. to set $CC and such) My idea was to switch back to intel instead of oneapi for those older versions given that the compilers are still there and could be used. Especially as EB automatically enables OneAPI compilers, so it's not always a user choice which would make the build fail here. But this could too be confusing and avoiding the failure can be easily done by toolchainopts = {'oneapi': False} (once the conditions here are correct)

Having mixed oneAPI/classic EasyBuild defaults is IMO temporary (what was the reason in the first place?). For the Score-P EasyBlock, I'd suggest to stick with classic (toolchainopts = {'oneapi': False}) and --with-nocross-compiler-suite=intel and switch to oneAPI (toolchainopts = {'oneapi': True}) and --with-nocross-compiler-suite=oneapi for oneAPI releases starting with 2024.0.

We are happy to assists users that really need a mixed version, to manually install such a version using --with-custom-compilers. But I'm reluctant to hardcode a solution for mixed compilers if it is just temporary. I'm not reluctant to use OMPI_CC and friends to guarantee the same compilers for build-backend|mpi|shmem, though.

@Flamefire
Copy link
Contributor

This sounds to me like what we should use. Especially as "ifort compiler did support compiler instrumentation flags. Those were added to ifx in 2024.0."
So IMO the behavior currently used by EasyBuild (use new C/C++ compilers but old Fortran compiler by default) seems to match the best support, doesn't it?

This would provide the best compiler instrumentation support for oneAPI releases prior 2024.0. From 2024.0 on, using icx/icpx/ifx should be fine.

It looks like it is planned by EB to switch also the Fortran compiler to oneAPI for 2024.0, see easybuilders/easybuild-framework#4455

But Score-P's configure option --with-nocross-compiler-suite does support either intel, using the classic icc/icpc/ifort, or oneapi, using icx/icpx/ifx. There is no --with-nocross-compiler-suite option to select the oneAPI/classic mix icx/icpx/ifort.

To configure a oneAPI/classic mix, one can either use --with-custom-compiler-suite or ./configure CC=... MPICC=... SHMEMCC=... ..., see Score-P's INSTALL file for a complete list of environment variables.

That was my intention: Use the default compilers as set by EB in the configure via --with-custom-compiler-suite

On a no-cross-compile machine, configure uses all variables from compiler-nocross-<vendor>, compiler-mpi-<mpi>, compiler-shmem-<shmem>. The top-level configure puts them explicitly on the configure lines of the build-backend|mpi|shmem configures, thus overriding EasyBuild variables of the same name.

With moderate changes to Score-P/Scalasca/OTF2/CUBE* one could prepend to *FLAGS, but I don't see the benefit. Please elaborate if you disagree.

I was thinking when using --with-custom-compiler-suite then the configure won't set those flags and use the ones from EasyBuild. As the flags don't seem to be anything special this should work, shouldn't it?

Having mixed oneAPI/classic EasyBuild defaults is IMO temporary (what was the reason in the first place?). For the Score-P EasyBlock, I'd suggest to stick with classic (toolchainopts = {'oneapi': False}) and --with-nocross-compiler-suite=intel and switch to oneAPI (toolchainopts = {'oneapi': True}) and --with-nocross-compiler-suite=oneapi for oneAPI releases starting with 2024.0.

Reason was that the Fortran compiler wasn't yet considered ready (by Intel). I agree with the rest

@Thyre
Copy link

Thyre commented Feb 16, 2024

On a no-cross-compile machine, configure uses all variables from compiler-nocross-<vendor>, compiler-mpi-<mpi>, compiler-shmem-<shmem>. The top-level configure puts them explicitly on the configure lines of the build-backend|mpi|shmem configures, thus overriding EasyBuild variables of the same name.
With moderate changes to Score-P/Scalasca/OTF2/CUBE* one could prepend to *FLAGS, but I don't see the benefit. Please elaborate if you disagree.

I was thinking when using --with-custom-compiler-suite then the configure won't set those flags and use the ones from EasyBuild. As the flags don't seem to be anything special this should work, shouldn't it?

While I cannot confirm that it works (or not), having -diag-disable=10441 is necessary for the deprecated compilers (icc / icpc / ifort) beginning with Score-P v8.4, as those will fail the compiler instrumentation check if this flag is not present, which means that Score-P will not record any Enter/Exit for functions.

We use a special check which fails when any output on stderr is generated, which includes the deprecation remark

# AFS_COMPILE_IFELSE_WERROR(input, [action-if-true], [action-if-false])
# -------------------------------------------------------------------------------
# Convenient wrapper of AC_COMPILE_IFELSE macro which sets ac_[]_AC_LANG_ABBREV[]_werror_flag
# so that the compilation fails if there is any output on stderr.
#
# The remaining behavior matches the one found in AC_COMPILE_IFELSE.
#
AC_DEFUN([AFS_COMPILE_IFELSE_WERROR],[
    # Any non-null value works here
    afs_compile_ifelse_werror_[]_AC_LANG_ABBREV[]_werror_flag_save=${ac_[]_AC_LANG_ABBREV[]_werror_flag}
    ac_[]_AC_LANG_ABBREV[]_werror_flag=yes

    # Now call AC_COMPILE_IFELSE like one would normally do
    AC_COMPILE_IFELSE([$1], [$2], [$3])

    ac_[]_AC_LANG_ABBREV[]_werror_flag=${afs_compile_ifelse_werror_[]_AC_LANG_ABBREV[]_werror_flag_save}
])

@boegel boegel added this to the 4.x milestone Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants