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

Reworking CMake support #142

Merged
merged 9 commits into from
Jan 18, 2022
Merged

Conversation

benson31
Copy link
Collaborator

@benson31 benson31 commented Jan 5, 2022

These changes are primarily to support two pain points in our current CMake:

  1. When CMake added CUDA as a first-class language, it changed the way users are expected to interact with the various features of CUDA (.cu files vs "CUDA toolkit" stuff). We (I) never updated the Aluminum (or Hydrogen (see Update cmake to find_package(CUDAToolkit) Elemental#124) or LBANN) CMakeLists to address this.
  2. CMake has recently added first-class language support for HIP as well. This addresses some of the issues I've been having with the ROCm CMake code path, but not all. However, things are stabilizing both on the HIP/ROCm side of the CMakery as well as on the Kitware side such that it is worthwhile to do some updates at this time.

One additional note, mentioned in a comment, is that this bumps the minimum CMake version to 3.21. This is supported locally on our machines, as well as on the external machines that I use. Additionally, Spack can build 3.21, and it's open-source and easy to build for non-Spack users. The reason for this version is that, happily, it is both the version at which HIP was added as a first-class language and the version at which Kitware fixed a bug with the FindCUDAToolkit module that affects machines using the HPC SDK.

TODO:

  • Possibly update version? Maybe we do that as a separate commit, though?
  • Iron out ROCm fp16 compilation issue.
  • Fix the Aluminum export.

@benson31 benson31 added the HIP/ROCm Stuff related to HIP/ROCm support label Jan 5, 2022
@benson31 benson31 self-assigned this Jan 5, 2022
Copy link
Collaborator

@ndryden ndryden left a comment

Choose a reason for hiding this comment

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

Let's bump the version in a separate commit/PR.

test/test_utils.hpp Outdated Show resolved Hide resolved
cmake/FindCUB.cmake Outdated Show resolved Hide resolved
cmake/FindNCCL.cmake Outdated Show resolved Hide resolved
Copy link
Collaborator

@ndryden ndryden 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 now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HIP/ROCm Stuff related to HIP/ROCm support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants