Skip to content

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Jul 28, 2025

Summary:
This is a bit of an awkward transition point for the new and old
drivers. Previously AMDGPU uses this to generate offloading bundles, but
the new driver much prefers to output the file itself. This patch
changes the behavior to always respect --gpu-bundle-output instead of
having it be the default behavior. This means that we effectively get to
override the default new driver behavior with this flag now. This should
hoepfully fix some errors in the downstream comgr tests.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jul 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 28, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Joseph Huber (jhuber6)

Changes

Summary:
This is a bit of an awkward transition point for the new and old
drivers. Previously AMDGPU uses this to generate offloading bundles, but
the new driver much prefers to output the file itself. This patch
changes the behavior to always respect --gpu-bundle-output instead of
having it be the default behavior. This means that we effectively get to
override the default new driver behavior with this flag now. This should
hoepfully fix some errors in the downstream comgr tests.


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

2 Files Affected:

  • (modified) clang/lib/Driver/Driver.cpp (+6-5)
  • (modified) clang/test/Driver/hip-binding.hip (+8-1)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 853f694850ba8..ea73ee93d44d3 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4919,13 +4919,14 @@ Action *Driver::BuildOffloadingActions(Compilation &C,
   }
 
   // HIP code in device-only non-RDC mode will bundle the output if it invoked
-  // the linker.
+  // the linker or if the user explicitly requested it.
   bool ShouldBundleHIP =
-      HIPNoRDC && offloadDeviceOnly() &&
       Args.hasFlag(options::OPT_gpu_bundle_output,
-                   options::OPT_no_gpu_bundle_output, true) &&
-      !llvm::any_of(OffloadActions,
-                    [](Action *A) { return A->getType() != types::TY_Image; });
+                   options::OPT_no_gpu_bundle_output, false) ||
+      (HIPNoRDC && offloadDeviceOnly() &&
+       !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() && !ShouldBundleHIP)
diff --git a/clang/test/Driver/hip-binding.hip b/clang/test/Driver/hip-binding.hip
index d8b3f1e242018..4d15f9739ba7f 100644
--- a/clang/test/Driver/hip-binding.hip
+++ b/clang/test/Driver/hip-binding.hip
@@ -61,7 +61,7 @@
 // MULTI-D-ONLY-NEXT: # "amdgcn-amd-amdhsa" - "clang", inputs: ["[[INPUT]]"], output: "[[GFX90a:.+]]"
 // MULTI-D-ONLY-NEXT: # "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs: ["[[GFX90a]]"], output: "[[GFX90a_OUT:.+]]"
 //
-// RUN: not %clang -### --target=x86_64-linux-gnu --offload-new-driver -ccc-print-bindings -nogpulib -nogpuinc \
+// RUN: not %clang -### --target=x86_64-linux-gnu --offload-new-driver -ccc-print-bindings -nogpulib -nogpuinc -emit-llvm \
 // RUN:        --no-gpu-bundle-output --offload-arch=gfx90a --offload-arch=gfx908 --offload-device-only -c -o %t %s 2>&1 \
 // RUN: | FileCheck -check-prefix=MULTI-D-ONLY-NO-BUNDLE-O %s
 // MULTI-D-ONLY-NO-BUNDLE-O: error: cannot specify -o when generating multiple output files
@@ -75,6 +75,13 @@
 // MULTI-D-ONLY-O-NEXT: "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs: ["[[GFX90A_OBJ]]"], output: "[[GFX90A:.+]]"
 // MULTI-D-ONLY-O-NEXT: "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs: ["[[GFX908]]", "[[GFX90A]]"], output: "a.out"
 
+// RUN: %clang -### --target=x86_64-linux-gnu --offload-new-driver -ccc-print-bindings -nogpulib -nogpuinc -emit-llvm \
+// RUN:        --gpu-bundle-output --offload-arch=gfx90a --offload-arch=gfx908 --offload-device-only -c -o a.out %s 2>&1 \
+// RUN: | FileCheck -check-prefix=MULTI-D-ONLY-BC %s
+//      MULTI-D-ONLY-BC: "amdgcn-amd-amdhsa" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[GFX908_BC:.+]]"
+// MULTI-D-ONLY-BC-NEXT: "amdgcn-amd-amdhsa" - "clang", inputs: ["[[INPUT]]"], output: "[[GFX90A_BC:.+]]"
+// MULTI-D-ONLY-BC-NEXT: "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs: ["[[GFX908_BC]]", "[[GFX90A_BC]]"], output: "a.out"
+
 //
 // Check to ensure that we can use '-fsyntax-only' for HIP output with the new
 // driver.

Copy link
Contributor

@lamb-j lamb-j left a comment

Choose a reason for hiding this comment

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

fixes the downstream issue with -emit-llvm and --gpu-bundle-output, thanks!

Summary:
This is a bit of an awkward transition point for the new and old
drivers. Previously AMDGPU uses this to generate offloading bundles, but
the new driver much prefers to output the file itself. This patch
changes the behavior to always respect `--gpu-bundle-output` instead of
having it be the default behavior. This means that we effectively get to
override the default new driver behavior with this flag now. This should
hoepfully fix some errors in the downstream comgr tests.
@jhuber6 jhuber6 force-pushed the fix_amd_bundling branch from a4ce7af to 80a4f28 Compare July 28, 2025 16:37
@jhuber6 jhuber6 merged commit 2368be3 into llvm:main Jul 28, 2025
9 checks passed
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.

4 participants