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 minimum compiler versions + clean up #5323

Merged
merged 10 commits into from
Aug 18, 2022

Conversation

masterleinad
Copy link
Contributor

@masterleinad masterleinad commented Aug 10, 2022

Fixes #5285. This excludes ROCm/HIPCC and icpx. We'll update them later.

@masterleinad masterleinad changed the title Remove code guarded for older compiler versions Update minimum compiler versions + clean up Aug 11, 2022
Comment on lines +208 to +209
// FIXME Workaround for ICE with intel <=21 in Trilinos
#if (KOKKOS_COMPILER_INTEL <= 2100)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This corresponds to #5311.

core/src/Kokkos_Macros.hpp Outdated Show resolved Hide resolved
cmake/kokkos_compiler_id.cmake Outdated Show resolved Hide resolved
cmake/kokkos_compiler_id.cmake Show resolved Hide resolved
@PhilMiller
Copy link
Contributor

I'll check the Intel version number stuff

@PhilMiller
Copy link
Contributor

Intel 19.0.5:

[pbmille@kokkos-dev-2 ~]$ module load sems-intel/19.0.5

$ which icpc
/projects/sems/install/rhel7-x86_64/sems-compilers/tpl/intel/19.0.5/gcc/8.3.0/base/u4jh4al/compilers_and_libraries_2019.5.281/linux/bin/intel64/icpc

$ icpc --version
icpc (ICC) 19.0.5.281 20190815
Copyright (C) 1985-2019 Intel Corporation.  All rights reserved.

$ cmake -DCMAKE_CXX_COMPILER=icpc ~/repos/kokkos 
-- Setting default Kokkos CXX standard to 14
-- The CXX compiler identification is Intel 19.0.5.20190815
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /projects/sems/install/rhel7-x86_64/sems-compilers/tpl/intel/19.0.5/gcc/8.3.0/base/u4jh4al/compilers_and_libraries_2019.5.281/linux/bin/intel64/icpc - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Setting build type to 'RelWithDebInfo' as none was specified.
-- The project name is: Kokkos
-- SERIAL backend is being turned on to ensure there is at least one Host space. To change this, you must enable another host execution space and configure with -DKokkos_ENABLE_SERIAL=OFF or change CMakeCache.txt
-- Using -std=gnu++14 for C++14 extensions as feature
-- Built-in Execution Spaces:
--     Device Parallel: NoTypeDefined
--     Host Parallel: NoTypeDefined
--       Host Serial: SERIAL
-- 
-- Architectures:
-- Found TPLLIBDL: /usr/include  
-- Using internal desul_atomics copy
-- Kokkos Devices: SERIAL, Kokkos Backends: SERIAL
-- Configuring done
-- Generating done
-- Build files have been written to: /ascldap/users/pbmille/build/2022-08-11-icpc19

$ cmake -DCMAKE_CXX_STANDARD=17 -DCMAKE_CXX_COMPILER=icpc ~/repos/kokkos 
-- Setting default Kokkos CXX standard to 17
-- The CXX compiler identification is Intel 19.0.5.20190815
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /projects/sems/install/rhel7-x86_64/sems-compilers/tpl/intel/19.0.5/gcc/8.3.0/base/u4jh4al/compilers_and_libraries_2019.5.281/linux/bin/intel64/icpc - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Setting build type to 'RelWithDebInfo' as none was specified.
-- The project name is: Kokkos
-- SERIAL backend is being turned on to ensure there is at least one Host space. To change this, you must enable another host execution space and configure with -DKokkos_ENABLE_SERIAL=OFF or change CMakeCache.txt
-- Using -std=gnu++17 for C++17 extensions as feature
-- Built-in Execution Spaces:
--     Device Parallel: NoTypeDefined
--     Host Parallel: NoTypeDefined
--       Host Serial: SERIAL
-- 
-- Architectures:
-- Found TPLLIBDL: /usr/include  
-- Using internal desul_atomics copy
-- Kokkos Devices: SERIAL, Kokkos Backends: SERIAL
-- Configuring done
-- Generating done
-- Build files have been written to: /ascldap/users/pbmille/build/2022-08-11-icpc19

$ cat intel.cc 
#include <iostream>

int main() {
  std::cout << '"' << __INTEL_COMPILER << '"' << std::endl;
  return 0;
}

$ make CXX=icpc intel
icpc     intel.cc   -o intel

$ ./intel 
"1900"

@masterleinad
Copy link
Contributor Author

Intel 19.0.5:

Thanks!

@masterleinad masterleinad marked this pull request as ready for review August 11, 2022 21:28
@masterleinad
Copy link
Contributor Author

The only failure is

: [ RUN      ] default_exec.overlap_mdrange_policy
24: 
24: Kokkos::Cuda::initialize WARNING: Cuda is allocating into UVMSpace by default
24:                                   without setting CUDA_MANAGED_FORCE_DEVICE_ALLOC=1 or
24:                                   setting CUDA_VISIBLE_DEVICES.
24:                                   This could on multi GPU systems lead to severe performance"
24:                                   penalties.
24: 
24: Kokkos::Cuda::initialize WARNING: Cuda is allocating into UVMSpace by default
24:                                   without setting CUDA_MANAGED_FORCE_DEVICE_ALLOC=1 or
24:                                   setting CUDA_VISIBLE_DEVICES.
24:                                   This could on multi GPU systems lead to severe performance"
24:                                   penalties.
24: /var/jenkins/workspace/Kokkos/core/perf_test/PerfTest_ExecSpacePartitioning.cpp:395: Failure
24: Expected: (time_end) > (1.5 * time_overlap), actual: 0.00477586 vs 0.00496419
24: [  FAILED  ] default_exec.overlap_mdrange_policy (451 ms)

in CUDA-11.0-NVCC-C++17-RDC.

MESSAGE(FATAL_ERROR "${KOKKOS_MESSAGE_TEXT}")
ENDIF()
ELSEIF(KOKKOS_CXX_COMPILER_ID STREQUAL XL OR KOKKOS_CXX_COMPILER_ID STREQUAL XLClang)
MESSAGE(FATAL_ERROR "${KOKKOS_MESSAGE_TEXT}")
Copy link
Member

Choose a reason for hiding this comment

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

The list does not explicitly mention IBM XL
Should we say it is not supported?
Also should we have a flag to turn the error into a warning and continue processing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See 30fc3d6.

core/src/Kokkos_Macros.hpp Show resolved Hide resolved
core/src/impl/Kokkos_QuadPrecisionMath.hpp Show resolved Hide resolved
@PhilMiller
Copy link
Contributor

Other than Damien's remark about explicitly reporting that IBM XL is no longer supported, this all looks good to me.

We may want to audit the remaining #if compiler version checks throughout the code for similar mistakes of boolean logic, since they seemed to be quite frequent. We may even want to introduce helper macros to encapsulate the correct comparisons, to avoid those mistakes in the future.

Copy link
Contributor

@cz4rs cz4rs left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@PhilMiller PhilMiller marked this pull request as draft August 17, 2022 18:34
@masterleinad masterleinad marked this pull request as ready for review August 17, 2022 20:41
@masterleinad
Copy link
Contributor Author

Dropping XL is now part of #5349.

# TODO check if PGI accepts GNU style warnings
KOKKOS_INTERNAL_COMPILER_WARNINGS =
ifeq ($(KOKKOS_INTERNAL_COMPILER_CLANG), 1)
KOKKOS_INTERNAL_COMPILER_WARNINGS = -Wall -Wunused-parameter -Wshadow -pedantic -Wsign-compare -Wtype-limits -Wuninitialized
else
Copy link
Member

Choose a reason for hiding this comment

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

please format this way

ifeq (COND1)
  ...
else ifeq (COND2)
  ...
else ifeq (COND3)
...
else
  ...
endif

to improve readability

Copy link
Member

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

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

Looks good besides the format fix I asked for

@dalg24 dalg24 merged commit af4bc75 into kokkos:develop Aug 18, 2022
@masterleinad masterleinad mentioned this pull request Sep 7, 2022
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

Successfully merging this pull request may close these issues.

None yet

4 participants