Skip to content

Conversation

@pvelesko
Copy link
Contributor

Backport of #136412 to release/21.x

HIPSPV: Unbundle SDL

This fixes the issue of rdc linking static libraries with device code

This fixes the issue of rdc linking static libraries with device code

CHIP-SPV/chipStar#984

---------

Co-authored-by: Henry Linjamäki <[email protected]>
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Oct 31, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 31, 2025

@llvm/pr-subscribers-clang-driver

Author: Paulius Velesko (pvelesko)

Changes

Backport of #136412 to release/21.x

HIPSPV: Unbundle SDL

This fixes the issue of rdc linking static libraries with device code


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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/HIPSPV.cpp (+9)
  • (added) clang/test/Driver/hipspv-link-static-library.hip (+28)
diff --git a/clang/lib/Driver/ToolChains/HIPSPV.cpp b/clang/lib/Driver/ToolChains/HIPSPV.cpp
index 53649ca40d99b..90fec72e826f4 100644
--- a/clang/lib/Driver/ToolChains/HIPSPV.cpp
+++ b/clang/lib/Driver/ToolChains/HIPSPV.cpp
@@ -69,8 +69,17 @@ void HIPSPV::Linker::constructLinkAndEmitSpirvCommand(
 
   // Link LLVM bitcode.
   ArgStringList LinkArgs{};
+
   for (auto Input : Inputs)
     LinkArgs.push_back(Input.getFilename());
+
+  // Add static device libraries using the common helper function.
+  // This handles unbundling archives (.a) containing bitcode bundles.
+  StringRef Arch = getToolChain().getTriple().getArchName();
+  StringRef Target =
+      "generic"; // SPIR-V is generic, no specific target ID like -mcpu
+  tools::AddStaticDeviceLibsLinking(C, *this, JA, Inputs, Args, LinkArgs, Arch,
+                                    Target, /*IsBitCodeSDL=*/true);
   LinkArgs.append({"-o", TempFile});
   const char *LlvmLink =
       Args.MakeArgString(getToolChain().GetProgramPath("llvm-link"));
diff --git a/clang/test/Driver/hipspv-link-static-library.hip b/clang/test/Driver/hipspv-link-static-library.hip
new file mode 100644
index 0000000000000..03126ae589a09
--- /dev/null
+++ b/clang/test/Driver/hipspv-link-static-library.hip
@@ -0,0 +1,28 @@
+// Test HIPSPV static device library linking
+// REQUIRES: system-linux
+// UNSUPPORTED: system-windows
+
+// Create a dummy archive to test SDL linking
+// RUN: rm -rf %t && mkdir %t
+// RUN: touch %t/dummy.bc  
+// RUN: llvm-ar cr %t/libSDL.a %t/dummy.bc
+
+// Test that -l options are passed to llvm-link for --offload=spirv64
+// RUN: %clang -### --target=x86_64-linux-gnu --offload=spirv64 \
+// RUN:   --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
+// RUN:   -L%t -lSDL \
+// RUN: 2>&1 | FileCheck -check-prefix=SDL-LINK %s
+
+// Test that .a files are properly unbundled and passed to llvm-link  
+// RUN: %clang -### --target=x86_64-linux-gnu --offload=spirv64 \
+// RUN:   --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
+// RUN:   %t/libSDL.a \
+// RUN: 2>&1 | FileCheck -check-prefix=SDL-ARCHIVE %s
+
+// Verify that the input files are added before the SDL files in llvm-link command
+// This tests the ordering fix to match HIPAMD behavior
+// SDL-LINK: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libSDL.a" "-targets=hip-spirv64-unknown-unknown-unknown-generic" "-output=[[SDL_A:.*\.a]]" "-allow-missing-bundles"
+// SDL-LINK: "{{.*}}llvm-link" "{{.*}}.bc" "[[SDL_A]]" "-o"
+
+// SDL-ARCHIVE: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libSDL.a" "-targets=hip-spirv64-unknown-unknown-unknown-generic" "-output=[[SDL_A:.*\.a]]" "-allow-missing-bundles"  
+// SDL-ARCHIVE: "{{.*}}llvm-link" "{{.*}}.bc" "[[SDL_A]]" "-o"

@llvmbot
Copy link
Member

llvmbot commented Oct 31, 2025

@llvm/pr-subscribers-clang

Author: Paulius Velesko (pvelesko)

Changes

Backport of #136412 to release/21.x

HIPSPV: Unbundle SDL

This fixes the issue of rdc linking static libraries with device code


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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/HIPSPV.cpp (+9)
  • (added) clang/test/Driver/hipspv-link-static-library.hip (+28)
diff --git a/clang/lib/Driver/ToolChains/HIPSPV.cpp b/clang/lib/Driver/ToolChains/HIPSPV.cpp
index 53649ca40d99b..90fec72e826f4 100644
--- a/clang/lib/Driver/ToolChains/HIPSPV.cpp
+++ b/clang/lib/Driver/ToolChains/HIPSPV.cpp
@@ -69,8 +69,17 @@ void HIPSPV::Linker::constructLinkAndEmitSpirvCommand(
 
   // Link LLVM bitcode.
   ArgStringList LinkArgs{};
+
   for (auto Input : Inputs)
     LinkArgs.push_back(Input.getFilename());
+
+  // Add static device libraries using the common helper function.
+  // This handles unbundling archives (.a) containing bitcode bundles.
+  StringRef Arch = getToolChain().getTriple().getArchName();
+  StringRef Target =
+      "generic"; // SPIR-V is generic, no specific target ID like -mcpu
+  tools::AddStaticDeviceLibsLinking(C, *this, JA, Inputs, Args, LinkArgs, Arch,
+                                    Target, /*IsBitCodeSDL=*/true);
   LinkArgs.append({"-o", TempFile});
   const char *LlvmLink =
       Args.MakeArgString(getToolChain().GetProgramPath("llvm-link"));
diff --git a/clang/test/Driver/hipspv-link-static-library.hip b/clang/test/Driver/hipspv-link-static-library.hip
new file mode 100644
index 0000000000000..03126ae589a09
--- /dev/null
+++ b/clang/test/Driver/hipspv-link-static-library.hip
@@ -0,0 +1,28 @@
+// Test HIPSPV static device library linking
+// REQUIRES: system-linux
+// UNSUPPORTED: system-windows
+
+// Create a dummy archive to test SDL linking
+// RUN: rm -rf %t && mkdir %t
+// RUN: touch %t/dummy.bc  
+// RUN: llvm-ar cr %t/libSDL.a %t/dummy.bc
+
+// Test that -l options are passed to llvm-link for --offload=spirv64
+// RUN: %clang -### --target=x86_64-linux-gnu --offload=spirv64 \
+// RUN:   --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
+// RUN:   -L%t -lSDL \
+// RUN: 2>&1 | FileCheck -check-prefix=SDL-LINK %s
+
+// Test that .a files are properly unbundled and passed to llvm-link  
+// RUN: %clang -### --target=x86_64-linux-gnu --offload=spirv64 \
+// RUN:   --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
+// RUN:   %t/libSDL.a \
+// RUN: 2>&1 | FileCheck -check-prefix=SDL-ARCHIVE %s
+
+// Verify that the input files are added before the SDL files in llvm-link command
+// This tests the ordering fix to match HIPAMD behavior
+// SDL-LINK: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libSDL.a" "-targets=hip-spirv64-unknown-unknown-unknown-generic" "-output=[[SDL_A:.*\.a]]" "-allow-missing-bundles"
+// SDL-LINK: "{{.*}}llvm-link" "{{.*}}.bc" "[[SDL_A]]" "-o"
+
+// SDL-ARCHIVE: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libSDL.a" "-targets=hip-spirv64-unknown-unknown-unknown-generic" "-output=[[SDL_A:.*\.a]]" "-allow-missing-bundles"  
+// SDL-ARCHIVE: "{{.*}}llvm-link" "{{.*}}.bc" "[[SDL_A]]" "-o"

@nikic nikic added this to the LLVM 21.x Release milestone Oct 31, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Oct 31, 2025
@c-rhodes c-rhodes requested a review from AaronBallman November 3, 2025 09:14
@c-rhodes c-rhodes moved this from Needs Triage to Needs Review in LLVM Release Status Nov 3, 2025
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

Thanks! I'd like a bit more details on why this should be backported. (What's the need and why are these changes safe to land?)

@@ -0,0 +1,28 @@
// Test HIPSPV static device library linking
// REQUIRES: system-linux
// UNSUPPORTED: system-windows
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this unsupported on Windows?

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

Status: Needs Review

Development

Successfully merging this pull request may close these issues.

4 participants