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] Correctly omit bundling with the new driver #85842

Merged
merged 1 commit into from
Mar 20, 2024
Merged

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Mar 19, 2024

Summary:
The HIP phases do not emit the offload bundler output when we do not
invoke the final linker phase in device only mode. Check this propery.

@jhuber6 jhuber6 requested review from Artem-B and yxsamliu March 19, 2024 18:30
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Mar 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 19, 2024

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Joseph Huber (jhuber6)

Changes

Summary:
The HIP phases do not emit the offload bundler output when we do not
invoke the final linker phase in device only mode. Check this propery.


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

2 Files Affected:

  • (modified) clang/lib/Driver/Driver.cpp (+11-6)
  • (modified) clang/test/Driver/hip-phases.hip (+12)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 1daf588142b3b4..e7d57635e03208 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4641,17 +4641,22 @@ Action *Driver::BuildOffloadingActions(Compilation &C,
           DDep.add(*Input, *TCAndArch->first, TCAndArch->second.data(), Kind);
       OffloadActions.push_back(C.MakeAction<OffloadAction>(DDep, A->getType()));
 
+
       ++TCAndArch;
     }
   }
 
+  // HIP code in non-RDC mode will bundle the output if it invoked the linker.
+  bool ShouldBundleHIP =
+      C.isOffloadingHostKind(Action::OFK_HIP) &&
+      Args.hasFlag(options::OPT_gpu_bundle_output,
+                   options::OPT_no_gpu_bundle_output, true) &&
+      !Args.hasFlag(options::OPT_fgpu_rdc, options::OPT_fno_gpu_rdc, false) &&
+      !llvm::any_of(OffloadActions,
+                    [](Action *A) { return A->getType() != types::TY_Image; });
+
   // All kinds exit now in device-only mode except for non-RDC mode HIP.
-  if (offloadDeviceOnly() &&
-      (getFinalPhase(Args) == phases::Preprocess ||
-       !C.isOffloadingHostKind(Action::OFK_HIP) ||
-       !Args.hasFlag(options::OPT_gpu_bundle_output,
-                     options::OPT_no_gpu_bundle_output, true) ||
-       Args.hasFlag(options::OPT_fgpu_rdc, options::OPT_fno_gpu_rdc, false)))
+  if (offloadDeviceOnly() && !ShouldBundleHIP)
     return C.MakeAction<OffloadAction>(DDeps, types::TY_Nothing);
 
   if (OffloadActions.empty())
diff --git a/clang/test/Driver/hip-phases.hip b/clang/test/Driver/hip-phases.hip
index ca63d4304d3959..9be6a5577cbdc7 100644
--- a/clang/test/Driver/hip-phases.hip
+++ b/clang/test/Driver/hip-phases.hip
@@ -648,3 +648,15 @@
 // LTO-NEXT: 14: offload, "host-hip (x86_64-unknown-linux-gnu)" {2}, "device-hip (x86_64-unknown-linux-gnu)" {13}, ir
 // LTO-NEXT: 15: backend, {14}, assembler, (host-hip)
 // LTO-NEXT: 16: assembler, {15}, object, (host-hip)
+
+//
+// Test the new driver when not bundling
+//
+// RUN: %clang -### --target=x86_64-linux-gnu --offload-new-driver -ccc-print-phases \
+// RUN:        --offload-device-only --offload-arch=gfx90a -emit-llvm -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix=DEVICE-ONLY %s
+//      DEVICE-ONLY: 0: input, "[[INPUT:.+]]", hip, (device-hip, gfx1030)
+// DEVICE-ONLY-NEXT: 1: preprocessor, {0}, hip-cpp-output, (device-hip, gfx1030)
+// DEVICE-ONLY-NEXT: 2: compiler, {1}, ir, (device-hip, gfx1030)
+// DEVICE-ONLY-NEXT: 3: backend, {2}, ir, (device-hip, gfx1030)
+// DEVICE-ONLY-NEXT: 4: offload, "device-hip (amdgcn-amd-amdhsa:gfx1030)" {3}, none

Copy link

github-actions bot commented Mar 19, 2024

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

Summary:
The HIP phases do not emit the offload bundler output when we do not
invoke the final linker phase in device only mode. Check this propery.
@jhuber6 jhuber6 merged commit 357f00d into llvm:main Mar 20, 2024
4 checks passed
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
Summary:
The HIP phases do not emit the offload bundler output when we do not
invoke the final linker phase in device only mode. Check this propery.
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