[Offload] Change HSA header search order

[jhuber6]

Summary:
The HSA headers existed previously in include/hsa.h and were moved to
include/hsa/hsa.h in a later ROCm version. The include headers here
were originally designed to favor a newer one. However, this
unintentionally prevented the dyanmic HSA's hsa.h from being used if
both were present. This patch changes the order so it will be found
first.

Related to #95484.

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-offload

Author: Joseph Huber (jhuber6)

Changes

Summary:
The HSA headers existed previously in include/hsa.h and were moved to
include/hsa/hsa.h in a later ROCm version. The include headers here
were originally designed to favor a newer one. However, this
unintentionally prevented the dyanmic HSA's hsa.h from being used if
both were present. This patch changes the order so it will be found
first.

Related to #95484.

---

Full diff: https://github.com/llvm/llvm-project/pull/95769.diff

1 Files Affected:

• (modified) offload/plugins-nextgen/amdgpu/src/rtl.cpp (+4-4)

diff --git a/offload/plugins-nextgen/amdgpu/src/rtl.cpp b/offload/plugins-nextgen/amdgpu/src/rtl.cpp
index 26bca4a3674bd..f16d150720206 100644
--- a/offload/plugins-nextgen/amdgpu/src/rtl.cpp
+++ b/offload/plugins-nextgen/amdgpu/src/rtl.cpp
@@ -57,12 +57,12 @@
 #endif
 
 #if defined(__has_include)
-#if __has_include("hsa/hsa.h")
-#include "hsa/hsa.h"
-#include "hsa/hsa_ext_amd.h"
-#elif __has_include("hsa.h")
+#if __has_include("hsa.h")
 #include "hsa.h"
 #include "hsa_ext_amd.h"
+#elif __has_include("hsa/hsa.h")
+#include "hsa/hsa.h"
+#include "hsa/hsa_ext_amd.h"
 #endif
 #else
 #include "hsa/hsa.h"

[shiltian]

I think https://discourse.llvm.org/t/18-1-7-released/79433 said there would be no LLVM 18 unless there is serious issue. Not sure this would be counted as one. @tstellar

[jhuber6]

I think https://discourse.llvm.org/t/18-1-7-released/79433 said there would be no LLVM 18 unless there is serious issue. Not sure this would be counted as one. @tstellar

Alright, so we'll just carry this fix into 19.x then.

[shiltian]

I'm actually not sure why the order makes a difference here? Is the issue caused by existence of include/hsa/hsa.h and include/hsa.h and they are actually different versions?

[jhuber6]

I'm actually not sure why the order makes a difference here? Is the issue caused by existence of include/hsa/hsa.h and include/hsa.h and they are actually different versions?

We add dyanic_hsa/has.h to the include path. If the user has hsa installed on the system it will find hsa/hsa.h first even when we intended to use the dyanmic_hsa version.

[shiltian]

I think this sounds very hacky. Does it work if we do target_include_directories(omptarget.rtl.amdgpu BEFORE PRIVATE dynamic_hsa)? This will make sure that the dyanic_hsa will be the first one to be listed in -I.

[jhuber6]

I think this sounds very hacky. Does it work if we do target_include_directories(BEFORE omptarget.rtl.amdgpu PRIVATE dynamic_hsa)? This will make sure that the dyanic_hsa will be the first one to be listed in -I.

No, because the logic in the file explicitly includes hsa/hsa.h if it exists instead of anything else.

[shiltian]

I see the issue. LG.