Skip to content

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
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