Use HIP as language in CMake #3646

quantumsteve · 2021-12-08T20:41:52Z

Please review the developer documentation
on the wiki of this project that contains help and requirements.

Proposed changes

Describe what this PR changes and why. If it closes an issue, link to it here
with a supported keyword.

Fixes #3581

What type(s) of changes does this code introduce?

Delete the items that do not apply

Code style update (formatting, renaming)
Refactoring (no functional changes, no api changes)
Build related changes

Does this introduce a breaking change?

Yes (requires CMake 3.21 or later)

What systems has this change been tested on?

Checklist

Update the following with a yes where the items apply. If you're unsure about any of them, don't hesitate to ask. This is
simply a reminder of what we are going to look for before merging your code.

Yes. This PR is up to date with current the current state of 'develop'
No. Documentation has been added (if appropriate)

Signed-off-by: Steven Hahn <hahnse@ornl.gov>

qmc-robot · 2021-12-08T20:42:02Z

Can one of the admins verify this patch?

quantumsteve · 2021-12-08T20:46:00Z

@ye-luo I'm getting the following warnings when building and suspect it's related to CMAKE_HIP_ARCHITECTURES.

clang-13: warning: argument unused during compilation: '--offload-arch=gfx906' [-Wunused-command-line-argument]
clang-13: warning: argument unused during compilation: '--offload-arch=gfx908' [-Wunused-command-line-argument]

prckent · 2021-12-08T21:04:52Z

Is this OK on spock?

ye-luo · 2021-12-08T21:10:48Z

@quantumsteve could you paste your cmake line here? I mean on a regular workstation not spock.

I got a strange error

cmake -DCMAKE_C_COMPILER=/opt/rocm/llvm/bin/clang -DCMAKE_CXX_COMPILER=/opt/rocm/llvm/bin/clang++ -DQMC_MPI=0 -DENABLE_CUDA=ON -DQMC_CUDA2HIP=ON ..
...
-- Check for working HIP compiler: /opt/rocm-4.5.0/llvm/bin/clang++
-- Check for working HIP compiler: /opt/rocm-4.5.0/llvm/bin/clang++ - broken
CMake Error at /home/packages/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-9.3.0/cmake-3.21.4-t47xgwhz7ql3mvemzhzc7zwsrly6na3s/share/cmake-3.21/Modules/CMakeTestHIPCompiler.cmake:65 (message):
  The HIP compiler

    "/opt/rocm-4.5.0/llvm/bin/clang++"

  is not able to compile a simple test program.

  It fails with the following output:

    Change Dir: /home/yeluo/opt/cleanup/qmcpack/build_R7_aompbuild_offload_cuda2hip_real_check/CMakeFiles/CMakeTmp
    
    Run Build Command(s):/usr/bin/make -f Makefile cmTC_90a18/fast && /usr/bin/make  -f CMakeFiles/cmTC_90a18.dir/build.make CMakeFiles/cmTC_90a18.dir/build
    make[1]: Entering directory '/home/yeluo/opt/cleanup/qmcpack/build_R7_aompbuild_offload_cuda2hip_real_check/CMakeFiles/CMakeTmp'
    Building HIP object CMakeFiles/cmTC_90a18.dir/testHIPCompiler.hip.o
    /opt/rocm-4.5.0/llvm/bin/clang++ -D__HIP_ROCclr__=1 -I/opt/rocm-4.5.0/hip/include -I/opt/rocm-4.5.0/include -isystem /opt/rocm-4.5.0/hip/../include -isystem /opt/rocm-4.5.0/llvm/lib/clang/13.0.0 -fopenmp--cuda-host-only  --offload-arch=gfx906 -mllvm -amdgpu-early-inline-all=true -mllvm -amdgpu-function-calls=false -o CMakeFiles/cmTC_90a18.dir/testHIPCompiler.hip.o -x hip -c /home/yeluo/opt/cleanup/qmcpack/build_R7_aompbuild_offload_cuda2hip_real_check/CMakeFiles/CMakeTmp/testHIPCompiler.hip
    clang-13: error: unknown argument: '-fopenmp--cuda-host-only'
    make[1]: *** [CMakeFiles/cmTC_90a18.dir/build.make:78: CMakeFiles/cmTC_90a18.dir/testHIPCompiler.hip.o] Error 1
    make[1]: Leaving directory '/home/yeluo/opt/cleanup/qmcpack/build_R7_aompbuild_offload_cuda2hip_real_check/CMakeFiles/CMakeTmp'
    make: *** [Makefile:127: cmTC_90a18/fast] Error 2

no idea where -fopenmp--cuda-host-only si from

quantumsteve · 2021-12-08T22:46:41Z

@ye-luo I had some trouble getting the spacing right between arguments in CMAKE_HIP_FLAGS. Expect it should be -fopenmp --cuda-host-only

Signed-off-by: Steven Hahn <hahnse@ornl.gov>

quantumsteve · 2021-12-08T22:55:16Z

Last commit fixes that issue on Nitrogen. @prckent I haven't tried building on spock.

Using the same cmake configuration as GitHub Actions:

cmake -GNinja -DCMAKE_C_COMPILER=/opt/rocm/llvm/bin/clang -DCMAKE_CXX_COMPILER=/opt/rocm/llvm/bin/clang++ -DQMC_MPI=0 -DENABLE_CUDA=ON -DQMC_CUDA2HIP=ON -DCMAKE_PREFIX_PATH=/opt/OpenBLAS/0.3.18 -DQMC_COMPLEX=0 -DQMC_MIXED_PRECISION=1 -DCMAKE_BUILD_TYPE=RelWithDebInfo ..

CMakeLists.txt

Signed-off-by: Steven Hahn <hahnse@ornl.gov>

markdewing · 2021-12-09T21:36:27Z

I'm getting some linking errors

ld.lld: error: undefined symbol: __tgt_target_data_update_mapper
>>> referenced by test_ompBLAS.cpp
>>>               CMakeFiles/test_omptarget_blas.dir/test_ompBLAS.cpp.o:(void qmcplusplus::test_gemv<float>(int, int, char))

This is on my desktop machine with the aomp 14.0 compiler and rocm 4.5.0. I did compile the aomp offload version successfully a few weeks ago.

quantumsteve · 2021-12-09T21:45:54Z

CMake/ClangCompilers.cmake

@@ -56,6 +56,9 @@ if(QMC_OMP)
  endif()
  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fopenmp")
  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fopenmp")
+  if(ENABLE_ROCM)


@markdewing Does adding another variable to this check fix the linking error your experiencing?

I had offload enabled. If I disable that, then it works.

markdewing · 2021-12-09T21:46:33Z

Nevermind ... I was confused about what this support was for. It's not for the offload version.

ye-luo · 2021-12-09T22:02:12Z

Had no luck with gcc

cmake -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ -DENABLE_CUDA=ON -DQMC_CUDA2HIP=ON -DQMC_MPI=OFF ..

it tries to link programs with clang instead of gcc.
Got messages like

[100%] Linking HIP executable test_CUDA

which is expected host code.

quantumsteve · 2021-12-10T15:45:33Z

Had no luck with gcc
cmake -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ -DENABLE_CUDA=ON -DQMC_CUDA2HIP=ON -DQMC_MPI=OFF ..
it tries to link programs with clang instead of gcc. Got messages like
[100%] Linking HIP executable test_CUDA
which is expected host code.

This test comment seems to suggest that the link language for mixed targets should always be HIP 😕
https://gitlab.kitware.com/cmake/cmake/-/blob/master/Tests/HIP/MixedLanguage/CMakeLists.txt

ye-luo · 2021-12-10T15:57:22Z

HIP is only part of the program. Having HIP compiler (and its flags) to take over linking is just so wrong.

quantumsteve · 2021-12-10T17:53:46Z

HIP is only part of the program. Having HIP compiler (and its flags) to take over linking is just so wrong.

HIP, CXX, and CUDA have values of 90, 30, and 15.
https://gitlab.kitware.com/cmake/cmake/-/blob/a6fa3fa136c291c36aefbc79b62995a63bf9107b/Modules/CMakeHIPCompiler.cmake.in#L27

Signed-off-by: Steven Hahn <hahnse@ornl.gov>

ye-luo · 2021-12-11T19:54:40Z

Unfortunately, I found the current ROCm 4.5 release is bad in many ways for adopting HIP as a language. I have to park this PR for the moment. ROCm/ROCm#1259 (comment)

quantumsteve · 2021-12-11T21:02:50Z

@ye-luo Thank you for reviewing this. I'll move onto other issues while we wait for a more usable ROCm release.

PDoakORNL · 2021-12-17T15:58:06Z

temporarily close or mark this [WIP] it's clutter up the PR list.

ye-luo · 2022-11-28T18:01:24Z

Test this please

ye-luo · 2022-11-28T20:02:59Z

Test this please

This reverts commit 15d499a.

ye-luo · 2022-11-28T22:01:25Z

Test this please

ye-luo · 2022-11-28T22:10:25Z

@quantumsteve @markdewing I think it is good now. We can use HIP as a cmake language. Since I did many changes. I should not approval my own changes. So please do another round of review.

ye-luo · 2022-11-29T20:52:03Z

@prckent please review the documentation update. crusher recipe has been updated to use rocm 5.3.0

CMakeLists.txt

tests/test_automation/nightly_test_scripts/nightly_ornl.sh

quantumsteve · 2022-11-30T16:03:31Z

@quantumsteve @markdewing I think it is good now. We can use HIP as a cmake language. Since I did many changes. I should not approval my own changes. So please do another round of review.

LGTM, though GitHub won't let me approve a PR I'm the original author of.

ye-luo · 2022-11-30T23:13:25Z

Confirmed good with rocm 5.4.0 as well.

ye-luo · 2022-11-30T23:14:03Z

Test this please

quantumsteve added 3 commits December 8, 2021 13:41

Remove hip_add_library

34ea8e8

Signed-off-by: Steven Hahn <hahnse@ornl.gov>

remove unnecessary lines

905ba2c

Signed-off-by: Steven Hahn <hahnse@ornl.gov>

Use CMAKE HIP language variables

35f4a27

Signed-off-by: Steven Hahn <hahnse@ornl.gov>

quantumsteve requested a review from ye-luo December 8, 2021 20:41

Missing space after -fopenmp

ba8878e

Signed-off-by: Steven Hahn <hahnse@ornl.gov>

markdewing reviewed Dec 9, 2021

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

Message contained incorrect CMake version

d609998

Signed-off-by: Steven Hahn <hahnse@ornl.gov>

quantumsteve commented Dec 9, 2021

View reviewed changes

quantumsteve and others added 3 commits December 10, 2021 13:05

Don't using HIP linker with C++ code

626050c

Signed-off-by: Steven Hahn <hahnse@ornl.gov>

Correct HIP source properties.

77442a6

Remove dependence on FindHIP.cmake

4add523

ye-luo marked this pull request as draft December 11, 2021 19:52

quantumsteve changed the title ~~Use HIP as language in CMake~~ [WIP] Use HIP as language in CMake Dec 17, 2021

Merge remote-tracking branch 'origin/develop' into cmake_hip_language

0a60b8e

ye-luo mentioned this pull request Jun 29, 2022

Add missing dependency in CMake. #4085

Merged

Still need to supply hip::xxx

ef7b5ad

ye-luo force-pushed the cmake_hip_language branch from 151536b to ef7b5ad Compare November 19, 2022 04:23

Merge branch 'develop' into cmake_hip_language

fd5feef

Point ROCM installation via ROCM_ROOT in CI.

15d499a

ye-luo added 2 commits November 28, 2022 15:57

Look for the hip runtime and libraries next to the compiler by default

735f64f

Revert "Point ROCM installation via ROCM_ROOT in CI."

560d34b

This reverts commit 15d499a.

ye-luo force-pushed the cmake_hip_language branch from 885eaac to 560d34b Compare November 28, 2022 21:59

ye-luo changed the title ~~[WIP] Use HIP as language in CMake~~ Use HIP as language in CMake Nov 28, 2022

ye-luo marked this pull request as ready for review November 28, 2022 22:09

ye-luo requested a review from prckent November 29, 2022 20:52

ye-luo added 4 commits November 29, 2022 15:53

Update crusher recipe. Minor expansion of Summit recipe.

2f59539

Update documentation.

e3822b3

Trap outdated HIP_ARCH option.

84abc52

Mention neede CMake version.

34ad46e

ye-luo force-pushed the cmake_hip_language branch from 8cdfad5 to 34ad46e Compare November 29, 2022 20:55

quantumsteve commented Nov 29, 2022

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

quantumsteve commented Nov 29, 2022

View reviewed changes

tests/test_automation/nightly_test_scripts/nightly_ornl.sh Show resolved Hide resolved

Merge branch 'develop' into cmake_hip_language

c2cfab1

ye-luo approved these changes Nov 30, 2022

View reviewed changes

ye-luo enabled auto-merge November 30, 2022 23:14

ye-luo merged commit 787b5ca into QMCPACK:develop Dec 1, 2022

quantumsteve deleted the cmake_hip_language branch December 1, 2022 14:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use HIP as language in CMake #3646

Use HIP as language in CMake #3646

quantumsteve commented Dec 8, 2021

qmc-robot commented Dec 8, 2021

quantumsteve commented Dec 8, 2021

prckent commented Dec 8, 2021

ye-luo commented Dec 8, 2021 •

edited

Loading

quantumsteve commented Dec 8, 2021

quantumsteve commented Dec 8, 2021

markdewing commented Dec 9, 2021

quantumsteve Dec 9, 2021

markdewing Dec 9, 2021

markdewing commented Dec 9, 2021 •

edited

Loading

ye-luo commented Dec 9, 2021 •

edited

Loading

quantumsteve commented Dec 10, 2021

ye-luo commented Dec 10, 2021

quantumsteve commented Dec 10, 2021 •

edited

Loading

ye-luo commented Dec 11, 2021

quantumsteve commented Dec 11, 2021

PDoakORNL commented Dec 17, 2021

ye-luo commented Nov 28, 2022

ye-luo commented Nov 28, 2022

ye-luo commented Nov 28, 2022

ye-luo commented Nov 28, 2022

ye-luo commented Nov 29, 2022 •

edited

Loading

quantumsteve commented Nov 30, 2022

ye-luo commented Nov 30, 2022

ye-luo commented Nov 30, 2022

Use HIP as language in CMake #3646

Use HIP as language in CMake #3646

Conversation

quantumsteve commented Dec 8, 2021

Proposed changes

What type(s) of changes does this code introduce?

Does this introduce a breaking change?

What systems has this change been tested on?

Checklist

qmc-robot commented Dec 8, 2021

quantumsteve commented Dec 8, 2021

prckent commented Dec 8, 2021

ye-luo commented Dec 8, 2021 • edited Loading

quantumsteve commented Dec 8, 2021

quantumsteve commented Dec 8, 2021

markdewing commented Dec 9, 2021

quantumsteve Dec 9, 2021

Choose a reason for hiding this comment

markdewing Dec 9, 2021

Choose a reason for hiding this comment

markdewing commented Dec 9, 2021 • edited Loading

ye-luo commented Dec 9, 2021 • edited Loading

quantumsteve commented Dec 10, 2021

ye-luo commented Dec 10, 2021

quantumsteve commented Dec 10, 2021 • edited Loading

ye-luo commented Dec 11, 2021

quantumsteve commented Dec 11, 2021

PDoakORNL commented Dec 17, 2021

ye-luo commented Nov 28, 2022

ye-luo commented Nov 28, 2022

ye-luo commented Nov 28, 2022

ye-luo commented Nov 28, 2022

ye-luo commented Nov 29, 2022 • edited Loading

quantumsteve commented Nov 30, 2022

ye-luo commented Nov 30, 2022

ye-luo commented Nov 30, 2022

ye-luo commented Dec 8, 2021 •

edited

Loading

markdewing commented Dec 9, 2021 •

edited

Loading

ye-luo commented Dec 9, 2021 •

edited

Loading

quantumsteve commented Dec 10, 2021 •

edited

Loading

ye-luo commented Nov 29, 2022 •

edited

Loading