Skip to content

[CUDA/HIP] propagate -cuid to a host-only compilation. #107483

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

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

Artem-B
Copy link
Member

@Artem-B Artem-B commented Sep 5, 2024

Right now we're bailing out too early, and -cuid does not get set for the host-only compilations.

@Artem-B Artem-B requested a review from yxsamliu September 5, 2024 22:48
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Sep 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 5, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Artem Belevich (Artem-B)

Changes

Right now we're bailing out too early, and -cuid does not get set for the host-only compilations.


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

2 Files Affected:

  • (modified) clang/lib/Driver/Driver.cpp (+6-6)
  • (modified) clang/test/Driver/hip-cuid.hip (+40)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 5b3783e20eabba..efe398dd531da7 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -3026,12 +3026,6 @@ class OffloadingActionBuilder final {
         // Set the flag to true, so that the builder acts on the current input.
         IsActive = true;
 
-        if (CompileHostOnly)
-          return ABRT_Success;
-
-        // Replicate inputs for each GPU architecture.
-        auto Ty = IA->getType() == types::TY_HIP ? types::TY_HIP_DEVICE
-                                                 : types::TY_CUDA_DEVICE;
         std::string CUID = FixedCUID.str();
         if (CUID.empty()) {
           if (UseCUID == CUID_Random)
@@ -3055,6 +3049,12 @@ class OffloadingActionBuilder final {
         }
         IA->setId(CUID);
 
+        if (CompileHostOnly)
+          return ABRT_Success;
+
+        // Replicate inputs for each GPU architecture.
+        auto Ty = IA->getType() == types::TY_HIP ? types::TY_HIP_DEVICE
+                                                 : types::TY_CUDA_DEVICE;
         for (unsigned I = 0, E = GpuArchList.size(); I != E; ++I) {
           CudaDeviceActions.push_back(
               C.MakeAction<InputAction>(IA->getInputArg(), Ty, IA->getId()));
diff --git a/clang/test/Driver/hip-cuid.hip b/clang/test/Driver/hip-cuid.hip
index ed7de782bba5ac..2e38c59ccf5ef1 100644
--- a/clang/test/Driver/hip-cuid.hip
+++ b/clang/test/Driver/hip-cuid.hip
@@ -58,6 +58,28 @@
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck -check-prefixes=COMMON,HEX %s
 
+// Check that cuid is propagated to the host-only compilation.
+// RUN: %clang -### -x hip \
+// RUN:   --target=x86_64-unknown-linux-gnu \
+// RUN:   --no-offload-new-driver \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   --offload-host-only \
+// RUN:   -c -nogpuinc -nogpulib -cuid=xyz_123 \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck -check-prefixes=HOST %s
+
+// Check that cuid is propagated to the device-only compilation.
+// RUN: %clang -### -x hip \
+// RUN:   --target=x86_64-unknown-linux-gnu \
+// RUN:   --no-offload-new-driver \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   --offload-device-only \
+// RUN:   -c -nogpuinc -nogpulib -cuid=xyz_123 \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck -check-prefixes=DEVICE %s
+
 // INVALID: invalid value 'invalid' in '-fuse-cuid=invalid'
 
 // COMMON: "-cc1"{{.*}} "-triple" "amdgcn-amd-amdhsa"
@@ -92,3 +114,21 @@
 // HEX-NOT: "-cuid=[[CUID]]"
 // COMMON-SAME: "-cuid=[[CUID2]]"
 // COMMON-SAME: "{{.*}}b.hip"
+
+// HOST: "-cc1"{{.*}} "-triple" "x86_64-unknown-linux-gnu"
+// HOST-SAME: "-cuid=[[CUID:xyz_123]]"
+// HOST-SAME: "{{.*}}a.cu"
+
+// HOST: "-cc1"{{.*}} "-triple" "x86_64-unknown-linux-gnu"
+// HOST-SAME: "-cuid=[[CUID]]"
+// HOST-SAME: "{{.*}}b.hip"
+
+// DEVICE: "-cc1"{{.*}} "-triple" "amdgcn-amd-amdhsa"
+// DEVICE-SAME: "-target-cpu" "gfx900"
+// DEVICE-SAME: "-cuid=[[CUID:xyz_123]]"
+// DEVICE-SAME: "{{.*}}a.cu"
+
+// DEVICE: "-cc1"{{.*}} "-triple" "amdgcn-amd-amdhsa"
+// DEVICE-SAME: "-target-cpu" "gfx900"
+// DEVICE-SAME: "-cuid=[[CUID]]"
+// DEVICE-SAME: "{{.*}}b.hip"

Copy link
Collaborator

@yxsamliu yxsamliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks

@Artem-B Artem-B merged commit 4a501a4 into llvm:main Sep 9, 2024
11 checks passed
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Dec 12, 2024
Right now we're bailing out too early, and `-cuid` does not get set for
the host-only compilations.

Change-Id: I07c5a2b23be66c2ba3bf97a2ebbd6169e05e37cf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

3 participants