Drop hsa prefix from include directories so users can #include <hsa/*>

[haampie]

It should really be preferred to use

#include <hsa/hsa.h>

over

#include <hsa.h>

and it seems downstream packages such as HIP are simply not using the hsa-runtime64::hsa-runtime64 target because it does not include the hsa/ prefix (?)

[haampie]

@skeelyamd does this repo accept PRs?

[skeelyamd]

It does and this PR is being considered, apologies for the silence. Ultimately the path to include is not up to me though. Another team must approve packaging, include, and directory layout changes. We've been trying to get to a flat include directory structure so that the packages can be configured for distro standard install paths more easily for a while. That process halted part way through to let other components start to use cmake targets. HIP should be using cmake targets at this point though they may not be using them completely correctly. Using the target is required for static builds and HIP does support this now. It seems it's time to resume this process.

There seems to be conflicting best practices on how a cmake project's include paths should be reported. Most that I've seen do not require a project specific path prefix as you suggest here. But there are notable exceptions. Is there a reference supporting this patch that should be considered? Changing the prefix breaks backward compatibility with existing code so is not something that we should do lightly.

[haampie]

We've been trying to get to a flat include directory structure so that the packages can be configured for distro standard install paths more easily for a while. That process halted part way through to let other components start to use cmake targets.

I think this is a good decision; among other things not all package managers support flat directory structures either (e.g. spack).

Is there a reference supporting this patch that should be considered?

https://cliutils.gitlab.io/modern-cmake/chapters/basics/structure.html is quite nice

Changing the prefix breaks backward compatibility with existing code so is not something that we should do lightly.

True, but there's many instances where scripts use their own find logic to detect hsa.h, and they use different includes. From spack stage $(spack dependents hsa-rocr-dev) and some grepping:

opencl-on-vdi/amdocl/cmake/modules/FindROCR.cmake                   :find_path(FIND_ROCR_INCLUDES hsa.h HINTS /opt/rocm/include /opt/rocm/hsa/include PATH_SUFFIXES hsa)
roctracer-dev/cmake_modules/env.cmake                               :find_file ( HSA_RUNTIME_INC "hsa/hsa.h" )
hip-4.0.0/hip-config.cmake.in                                       :find_path(HSA_HEADER hsa/hsa.h
hip-4.0.0/cmake/FindROCR.cmake                                      :find_path(FIND_ROCR_INCLUDES hsa.h HINTS /opt/rocm/include /opt/rocm/hsa/include PATH_SUFFIXES hsa)
rocm-debug-agent-4.0.0/cmake/modules/FindROCR.cmake                 :find_path(FIND_ROCR_INCLUDES hsa/hsa.h HINTS /opt/rocm/include /opt/rocm/hsa/include)
rocm-opencl-4.0.0/amdocl/cmake/modules/FindROCR.cmake               :find_path(FIND_ROCR_INCLUDES hsa.h HINTS /opt/rocm/include /opt/rocm/hsa/include PATH_SUFFIXES hsa)
rocprofiler-dev-4.0.0/roctracer/cmake_modules/env.cmake             :find_file ( HSA_RUNTIME_INC "hsa/hsa.h" )
rocprofiler-dev-4.0.0/cmake_modules/env.cmake                       :find_file ( HSA_RUNTIME_INC "hsa/hsa.h" )
roctracer-dev-4.0.0/cmake_modules/env.cmake                         :find_file ( HSA_RUNTIME_INC "hsa/hsa.h" )
hip-rocclr-4.0.0/cmake/modules/FindROCR.cmake                       :find_path(FIND_ROCR_INCLUDES hsa.h HINTS /opt/rocm/include /opt/rocm/hsa/include PATH_SUFFIXES hsa)
hip-rocclr-4.0.0/opencl-on-vdi/amdocl/cmake/modules/FindROCR.cmake  :find_path(FIND_ROCR_INCLUDES hsa.h HINTS /opt/rocm/include /opt/rocm/hsa/include PATH_SUFFIXES hsa)
atmi-4.0.0/src/cmake_modules/FindROCm.cmake                         :    hsa.h

these try to find hsa.h themselves and do not use the target. Some use this target

amd-llvm-project/openmp/libomptarget/plugins/hsa/CMakeLists.txt                      :  hsa-runtime64::hsa-runtime64
aomp-3.10.0/aomp-dir/amd-llvm-project/openmp/libomptarget/plugins/hsa/CMakeLists.txt :  hsa-runtime64::hsa-runtime64
hip-4.0.0/lpl_ca/CMakeLists.txt                                                      :target_link_libraries(ca PUBLIC hsa-runtime64::hsa-runtime64 )
hip-4.0.0/rocclr/CMakeLists.txt                                                      :    target_link_libraries(amdhip64 PRIVATE amdrocclr_static Threads::Threads dl hsa-runtime64::hsa-runtime64)
hip-4.0.0/rocclr/CMakeLists.txt                                                      :    target_link_libraries(amdhip64 PRIVATE Threads::Threads dl hsa-runtime64::hsa-runtime64 amd_comgr)
hip-rocclr-4.0.0/CMakeLists.txt                                                      :  target_link_libraries(amdrocclr_static PRIVATE hsa-runtime64::hsa-runtime64)
hip-rocclr-4.0.0/device/rocm/CMakeLists.txt                                          :    $<TARGET_PROPERTY:hsa-runtime64::hsa-runtime64,INTERFACE_INCLUDE_DIRECTORIES>)
rccl-4.0.0/test/CMakeLists.txt                                                       :    target_link_libraries(UnitTests PRIVATE amdhip64 amd_comgr hsa-runtime64::hsa-runtime64)
rocm-bandwidth-test-4.0.0/CMakeLists.txt                                             :   target_link_libraries(${TEST_NAME} PRIVATE hsa-runtime64::hsa-runtime64)
rocminfo-4.0.0/CMakeLists.txt                                                        :target_link_libraries(${ROCMINFO_EXE} hsa-runtime64::hsa-runtime64)

and in terms of includes of hsa/hsa.h:

aomp-3.10.0/examples/cloc/vector_copy/vector_copy.cpp                                                       :#include <hsa/hsa.h>
hip-4.0.0/include/hip/hcc_detail/hip_runtime_api.h                                                          :#include <hsa/hsa.h>
hip-4.0.0/include/hip/hcc_detail/hsa_helpers.hpp                                                            :#include <hsa/hsa.h>
hip-4.0.0/include/hip/hcc_detail/program_state.hpp                                                          :#include <hsa/hsa.h>
hip-4.0.0/src/code_object_bundle.inl                                                                        :#include <hsa/hsa.h>
hip-4.0.0/src/hip_hcc_internal.h                                                                            :#include <hsa/hsa.h>
hip-4.0.0/src/hip_memory.cpp                                                                                :#include "hsa/hsa.h"
hip-4.0.0/src/hip_module.cpp                                                                                :#include <hsa/hsa.h>
hip-4.0.0/src/hip_texture.cpp                                                                               :#include "hsa/hsa.h"
hip-4.0.0/src/hiprtc.cpp                                                                                    :#include <hsa/hsa.h>
hip-4.0.0/src/program_state.cpp                                                                             :#include <hsa/hsa.h>
hip-4.0.0/src/program_state.inl                                                                             :#include <hsa/hsa.h>
llvm-project/openmp/libomptarget/plugins/hsa/impl/hostcall.cpp                                              :#include <hsa/hsa.h>
rccl-4.0.0/src/graph/topo.cc                                                                                :#include <hsa/hsa.h>
rccl-4.0.0/src/graph/xml.cc                                                                                 :#include <hsa/hsa.h>
rccl-4.0.0/src/graph/xml.cc                                                                                 :#include <hsa/hsa.h>
rccl-4.0.0/src/transport/p2p.cc                                                                             :#include <hsa/hsa.h>
rocm-dbgapi-4.0.0/src/dispatch.h                                                                            :#include <hsa/hsa.h>
rocm-dbgapi-4.0.0/src/queue.cpp                                                                             :#include <hsa/hsa.h>
rocm-debug-agent-4.0.0/src/debug_agent.cpp                                                                  :#include <hsa/hsa.h>
rocm-openmp-extras-4.0.0/examples/cloc/vector_copy/vector_copy.cpp                                          :#include <hsa/hsa.h>
rocm-openmp-extras-4.0.0/rocm-openmp-extras/llvm-project/openmp/libomptarget/plugins/hsa/impl/hostcall.cpp  :#include <hsa/hsa.h>
rocm-validation-suite-4.0.0/pebb.so/include/worker_b2b.h                                                    :#include "hsa/hsa.h"
rocm-validation-suite-4.0.0/pebb.so/src/action_run.cpp                                                      :#include "hsa/hsa.h"
rocm-validation-suite-4.0.0/pebb.so/src/action.cpp                                                          :#include "hsa/hsa.h"
rocm-validation-suite-4.0.0/pqt.so/include/action.h                                                         :#include "hsa/hsa.h"
rocm-validation-suite-4.0.0/pqt.so/include/worker_b2b.h                                                     :#include "hsa/hsa.h"
rocm-validation-suite-4.0.0/src/rvshsa.cpp                                                                  :#include "hsa/hsa.h"
rocprofiler-dev-4.0.0/roctracer/inc/ext/hsa_rt_utils.hpp                                                    :#include <hsa/hsa.h>
roctracer-dev-4.0.0/inc/ext/hsa_rt_utils.hpp                                                                :#include <hsa/hsa.h>
roctracer-dev/inc/ext/hsa_rt_utils.hpp                                                                      :#include <hsa/hsa.h>

[keryell]

Any progress on this?