Skip to content

Add HIP support for the unit tests#894

Open
HanatoK wants to merge 12 commits intoColvars:masterfrom
HanatoK:colvars_gpu_hip
Open

Add HIP support for the unit tests#894
HanatoK wants to merge 12 commits intoColvars:masterfrom
HanatoK:colvars_gpu_hip

Conversation

@HanatoK
Copy link
Copy Markdown
Member

@HanatoK HanatoK commented Nov 25, 2025

This is only a draft PR at present. I haven't figured out how to use libhipcxx for cuda::std::array.

Update:

I have implemented colvars_gpu::array1d to avoid the use of libhipcxx. The unit tests can now be run on HIP. You can build the run_colvars_test_cuda on HIP with the following commands:

cd tests/functional_gpu
mkdir build
cd build
cmake ../ -DGPU_TYPE=HIP -DCMAKE_CXX_COMPILER:PATH=/opt/rocm/bin/amdclang++ -DCMAKE_PREFIX_PATH=/opt/rocm -DCMAKE_BUILD_TYPE=Release

and then run make and make test. The test passed on my laptop with Radeon 610M and also Frontier with MI250X (requires `-DCMAKE_HIP_ARCHITECTURES="gfx90a").

@HanatoK HanatoK changed the title build: add HIP support Add HIP support for the unit tests Apr 14, 2026
@HanatoK HanatoK requested review from giacomofiorin and jhenin April 14, 2026 14:32
@HanatoK HanatoK marked this pull request as ready for review April 14, 2026 14:34
Copy link
Copy Markdown
Member

@jhenin jhenin left a comment

Choose a reason for hiding this comment

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

This is sensitive to the toolchain version (of course). With ROCm 6.4.3 I get "use of undeclared identifier '__syncwarp'", but with ROCm 7.1.1 I get into a kind dependency hell on the machine I'm trying (it loads the wrong compiler). Not an issue with the PR itself, but that explains why I couldn't finish the test just yet.

check_language(CUDA)
enable_language(CUDA)
if(NOT EXISTS ${COLVARS_SOURCE_DIR})
set(COLVARS_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../.." CACHE STRING "Colvars source code directory")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not working for me when cmake is run from within tests/functional_gpu/build - it's missing one level to go back to the Colvars root. I realized the same issue exists in namd/cudaglobalmaster/CMakeLists.txt. Unless I am missing something...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I am not what the best way to detect the Colvars source directory is. Could you specify -DCOLVARS_SOURCE_DIR= explicitly?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I can do that but I am wondering if this default path is the correct one. Does the default work for you?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It works for me. If I understand correctly, "CMAKE_CURRENT_SOURCE_DIR" should point to the directory containing the "CMakeLists.txt" being processed.

@HanatoK
Copy link
Copy Markdown
Member Author

HanatoK commented Apr 14, 2026

This is sensitive to the toolchain version (of course). With ROCm 6.4.3 I get "use of undeclared identifier '__syncwarp'", but with ROCm 7.1.1 I get into a kind dependency hell on the machine I'm trying (it loads the wrong compiler). Not an issue with the PR itself, but that explains why I couldn't finish the test just yet.

The code should be OK with ROCm 7.2.0 and 7.2.2.

@HanatoK
Copy link
Copy Markdown
Member Author

HanatoK commented Apr 16, 2026

This is sensitive to the toolchain version (of course). With ROCm 6.4.3 I get "use of undeclared identifier '__syncwarp'", but with ROCm 7.1.1 I get into a kind dependency hell on the machine I'm trying (it loads the wrong compiler). Not an issue with the PR itself, but that explains why I couldn't finish the test just yet.

I have tried to make the code compatible with ROCm 6.4.2 in the two latest commits.

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.

2 participants