Use hipcc from hip_HIPCC_EXECUTABLE

[haampie]

Since hip-config.cmake.in is already aware of where hipcc resides,
query the hipcc version from this variable, and if it doesn't work,
throw a hard error.

[haampie]

@pfultz2, could you please review this? My take is that *-config.cmake files should do a minimal amount of searching whenever they can know paths upfront. This change ensures we always use hipcc that was created by the build, and detect clang from the hipcc --version.

More importantly, if things go wrong, it throws a fatal configuration error, instead of silently ignoring the issue.

[pfultz2]

It shouldn't use hipcc to find clang root since it doesnt know where the compiler is unless it is in /opt/rocm/llvm. If the user sets CMAKE_CXX_COMPILER to a local clang then the clang root should point to this local directory(whereas hipcc wont).

We do search using hipcc, but only when the user has set CMAKE_CXX_COMPILER to hipcc instead of the compiler. Setting it to the compiler is preferred(as documented here) to ensure the correct compiler and headers are used.

[haampie]

Thanks. Let me elaborate: I'm installing llvm-amdgpu + hip through spack. spack knows clang's prefix, and it sets it as an env variable so that hipcc is usable. So yes, this is going in circles, since hipcc requires you to define the HIP_CLANG_PATH env variable.

However, in my project I don't want to set CMAKE_CXX_COMPILER to hipcc or clang. It is using gcc as the default compiler, and only *.cu sources are compiled with hipcc. This is done using find_package(HIP) and find_packge(hip) combined:

find_package(hip CONFIG REQUIRED)
find_package(HIP MODULE REQUIRED)
HIP_ADD_LIBRARY(mylib SHARED ... sources.cu  sources.cc sources.f ...)
target_link_libraries(mylib PUBLIC ... hip::host ...)

Now it would be great if linking to hip::host "just worked", but as it is right now it fails because I don't have rocm installed in the standard location nor am I using hipcc or clang as default compiler.

[haampie]

How about making clang a true dependency of hip so that hip-config.cmake just add something like:

find_package(clang REQUIRED HINTS @generated_path_to_clang@)

so that you don't have to jump through hoops to detect HIP_CLANG_ROOT?

[pfultz2]

Now it would be great if linking to hip::host "just worked", but as it is right now it fails because I don't have rocm installed in the standard location nor am I using hipcc or clang as default compiler.

What is the error? hip::host should work with gcc as hip::host doesn't need any clang headers. If this is failing, then we should fix that by making sure it doesn't error when there is no clang compiler or hipcc installed.

[pfultz2]

so that you don't have to jump through hoops to detect HIP_CLANG_ROOT?

This wont work as it could find a different clang then the one specified by the user. To use hip::device you need CMAKE_CXX_COMPILER set to a device compiler(but hip::host should work with vanilla gcc).

[haampie]

I'm not getting an error, but the gcc compiler invocation ends up with -LHIP_CLANG_INCLUDE_PATH-NOTFOUND/../lib/linux -lclang_rt.builtins-x86_64 🙃 as a result of

  # Add support for __fp16 and _Float16, explicitly link with compiler-rt
  set_property(TARGET hip::host APPEND PROPERTY
    INTERFACE_LINK_LIBRARIES -L${HIP_CLANG_INCLUDE_PATH}/../lib/linux -lclang_rt.builtins-x86_64
  )

introduced by 56392b4

This is also related to an issue I've opened today: #2215 (comment)

[haampie]

This wont work as it could find a different clang then the one specified by the user.

Sure, but isn't the clang version fixed? hipcc and (AMD's fork of) clang are shipped together, and it's not recommended or possibly not guaranteed to work when you mix different rocm releases; I mean, you probably shouldn't use hipcc from 4.0.0 and clang from 3.10.0 etc.

Correct me if I'm wrong, but to me the situation looks very similar to compiling clang which depends on gcc. You have to configure LLVM to point to GCC through cmake -DGCC_INSTALL_PREFIX=/path/to/gcc and this is from then on fixed.

Similarly it would be easy to fix the clang version for hipcc through find_package(clang REQUIRED) in the HIP/CMakeLists.txt here, and generate find_package(clang REQUIRED HINTS @generatedpath@) in hip-config.cmake based on that.

[pfultz2]

Sure, but isn't the clang version fixed?

There could be multiple versions of clang installed not necessarily from rocm.

I'm not getting an error, but the gcc compiler invocation ends up with -LHIP_CLANG_INCLUDE_PATH-NOTFOUND/../lib/linux -lclang_rt.builtins-x86_64 🙃 as a result of

Yea, this should only be linked on hip::device as I believe its needed for __fp16 support on the GPU. If users need this on the host-side, then they should explicitly add it.

[haampie]

Yea, this should only be linked on hip::device as I believe its needed for __fp16 support on the GPU. If users need this on the host-side, then they should explicitly add it.

Ok, PR created: #2219

There could be multiple versions of clang installed not necessarily from rocm.

Now I fail to see the point of HIP_CLANG_INCLUDE_PATH. Why would you want to set INCLUDE_DIRECTORIES of the hip::device target pointing to the include directories of the current compiler? That's no news for the compiler since it's part of clang --print-search-dirs, right?

[haampie]

Hi @pfultz2, could you please consider the following use case:

The host compiler is necessarily GCC for all cpp/c/f sources (my package depends on Fortran code that uses GNU's OpenMP through gfortran, so my package needs gcc -fopenmp, not clang -fopenmp), but there is some device code that is compiled with hipcc using the FindHIP cmake module. Some non-device code still includes hip header files, so I would like to use target_link_libraries(mylib hip::host) to add appropriate defines (like HIP_PLATFORM_HCC=1).

Now when I do find_package(hip) for this use-case, hip-config.cmake has no clue about how to locate clang. It just ends up with HIP_CLANG_INCLUDE_PATH-NOTFOUND and this is otherwise ignored. It's not a fatal error or anything, but it just suggests that the current hip-config.cmake file is broken.

What's wrong with storing a path to the default llvm/clang hip was built with? Would also solve the annoying issue of hipcc having no clue where to find clang.

[mangupta]

Closing all existing pull requests that do not target the develop branch. In case this pull request is still valid, please rebase your changes against develop branch and resubmit.