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 Alpaka version, make HIP compilation work #504

Merged
merged 8 commits into from
Jan 18, 2024

Conversation

StewMH
Copy link
Contributor

@StewMH StewMH commented Nov 28, 2023

As with CUDA, change the source language when compiling Alpaka with the HIP backend. Also, update the version and adapt to API changes.

Less changes than I expected in the end.

@StewMH
Copy link
Contributor Author

StewMH commented Nov 28, 2023

cc @CrossR

Copy link
Contributor

@beomki-yeo beomki-yeo 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!

Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

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

Yeah, this all looks fine. Just please rebase your branch. And possibly remove the CMAKE_HIP_EXTENSIONS line.

CMakeLists.txt Outdated
@@ -14,6 +14,8 @@ set( CMAKE_CXX_EXTENSIONS FALSE CACHE BOOL "Disable (host) C++ extensions" )
set( CMAKE_CUDA_STANDARD 17 CACHE STRING "The (CUDA) C++ standard to use" )
set( CMAKE_CUDA_EXTENSIONS FALSE CACHE BOOL "Disable (CUDA) C++ extensions" )
set( CMAKE_SYCL_STANDARD 17 CACHE STRING "The (SYCL) C++ standard to use" )
set( CMAKE_HIP_STANDARD 17 CACHE STRING "The (HIP) C++ standard to use" )
set( CMAKE_HIP_EXTENSIONS FALSE CACHE BOOL "Disable (HIP) C++ extensions" )
Copy link
Member

Choose a reason for hiding this comment

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

Since we're using VecMem's HIP support here (Unless I'm very mistaken. Unfortunately the setup is not super clear on this, as I chose the same API for VecMem's HIP support that CMake implemented eventually.), this variable doesn't really do anything. That's why I also didn't add a CMAKE_SYCL_EXTENSIONS flag. As that wouldn't do anything either.

The variable is not harmful, but it may be well be removed...

Stewart Martin-Haugh stewart.martin-haugh@stfc.ac.uk and others added 2 commits January 17, 2024 14:47
@krasznaa krasznaa merged commit 3d591e8 into acts-project:main Jan 18, 2024
17 checks passed
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.

3 participants