Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HIP][Driver] Automatically include hipstdpar forwarding header #78915

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

AlexVlx
Copy link
Contributor

@AlexVlx AlexVlx commented Jan 21, 2024

The forwarding header used by hipstdpar on AMDGPU targets is now pacakged with rocThrust. This change augments the ROCm Driver component so that it can automatically pick up the packaged header iff the user hasn't overridden it via the dedicated flag.

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:AMDGPU clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jan 21, 2024
@AlexVlx AlexVlx removed clang Clang issues not falling into any other category backend:AMDGPU labels Jan 21, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 21, 2024

@llvm/pr-subscribers-backend-amdgpu
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Alex Voicu (AlexVlx)

Changes

The forwarding header used by hipstdpar on AMDGPU targets is now pacakged with rocThrust. This change augments the ROCm Driver component so that it can automatically pick up the packaged header iff the user hasn't overridden it via the dedicated flag.


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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/AMDGPU.cpp (+17-8)
  • (modified) clang/test/Driver/hipstdpar.c (+2-1)
diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index 56f06fc5fccb7e..8a88dba562c8c0 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -545,26 +545,35 @@ void RocmInstallationDetector::AddHIPIncludeArgs(const ArgList &DriverArgs,
   }
 
   const auto HandleHipStdPar = [=, &DriverArgs, &CC1Args]() {
-    if (!hasHIPStdParLibrary()) {
+    StringRef Inc = getIncludePath();
+    auto &FS = D.getVFS();
+
+    if (!hasHIPStdParLibrary())
+      if (!HIPStdParPathArg.empty() ||
+          !FS.exists(Inc + "/thrust/system/hip/hipstdpar/hipstdpar_lib.hpp")) {
       D.Diag(diag::err_drv_no_hipstdpar_lib);
       return;
     }
-    if (!HasRocThrustLibrary &&
-        !D.getVFS().exists(getIncludePath() + "/thrust")) {
+    if (!HasRocThrustLibrary && !FS.exists(Inc + "/thrust")) {
       D.Diag(diag::err_drv_no_hipstdpar_thrust_lib);
       return;
     }
-    if (!HasRocPrimLibrary &&
-        !D.getVFS().exists(getIncludePath() + "/rocprim")) {
+    if (!HasRocPrimLibrary && !FS.exists(Inc + "/rocprim")) {
       D.Diag(diag::err_drv_no_hipstdpar_prim_lib);
       return;
     }
-
     const char *ThrustPath;
     if (HasRocThrustLibrary)
       ThrustPath = DriverArgs.MakeArgString(HIPRocThrustPathArg);
     else
-      ThrustPath = DriverArgs.MakeArgString(getIncludePath() + "/thrust");
+      ThrustPath = DriverArgs.MakeArgString(Inc + "/thrust");
+
+    const char *HIPStdParPath;
+    if (hasHIPStdParLibrary())
+      HIPStdParPath = DriverArgs.MakeArgString(HIPStdParPathArg);
+    else
+      HIPStdParPath = DriverArgs.MakeArgString(StringRef(ThrustPath) +
+                                               "/system/hip/hipstdpar");
 
     const char *PrimPath;
     if (HasRocPrimLibrary)
@@ -573,7 +582,7 @@ void RocmInstallationDetector::AddHIPIncludeArgs(const ArgList &DriverArgs,
       PrimPath = DriverArgs.MakeArgString(getIncludePath() + "/rocprim");
 
     CC1Args.append({"-idirafter", ThrustPath, "-idirafter", PrimPath,
-                    "-idirafter", DriverArgs.MakeArgString(HIPStdParPathArg),
+                    "-idirafter", HIPStdParPath,
                     "-include", "hipstdpar_lib.hpp"});
   };
 
diff --git a/clang/test/Driver/hipstdpar.c b/clang/test/Driver/hipstdpar.c
index 69c5b177d170cd..2f48bf6b5cf1eb 100644
--- a/clang/test/Driver/hipstdpar.c
+++ b/clang/test/Driver/hipstdpar.c
@@ -5,7 +5,8 @@
 // XFAIL: target={{.*}}-scei{{.*}}
 // XFAIL: target={{.*}}-sie{{.*}}
 
-// RUN: not %clang -### --hipstdpar -nogpulib -nogpuinc --compile %s 2>&1 | \
+// RUN: not %clang -### --hipstdpar --hipstdpar-path=/does/not/exist -nogpulib \
+// RUN:   -nogpuinc --compile %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=HIPSTDPAR-MISSING-LIB %s
 // RUN: %clang -### --hipstdpar --hipstdpar-path=%S/Inputs/hipstdpar \
 // RUN:   --hipstdpar-thrust-path=%S/Inputs/hipstdpar/thrust \

Copy link

github-actions bot commented Jan 21, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:AMDGPU labels Jan 21, 2024
@AlexVlx AlexVlx merged commit 907f2a0 into llvm:main Jan 23, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AMDGPU clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants