[clang] [ClangLinkerWrapper] Refactor target ID sanitization for Windows file… (PR #168744)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 19 10:09:00 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-driver
Author: Yaxun (Sam) Liu (yxsamliu)
<details>
<summary>Changes</summary>
… names
Fix non-RDC mode HIP compilation for the new driver on Windows due to invalid temporary file names when offload arch is a target ID containing ':', which is invalid in file names on Windows.
Refactor the existing handling of ':' in file names on Windows from clang driver into a shared function sanitizeTargetIDInFileName in clang/Basic/TargetID.h. This function replaces ':' with '@' on Windows only, preserving the original behavior.
Update both clang/lib/Driver/Driver.cpp and
clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp to use this shared function, ensuring consistent handling across both tools.
---
Full diff: https://github.com/llvm/llvm-project/pull/168744.diff
5 Files Affected:
- (modified) clang/include/clang/Basic/TargetID.h (+5)
- (modified) clang/lib/Basic/TargetID.cpp (+10)
- (modified) clang/lib/Driver/Driver.cpp (+1-6)
- (modified) clang/test/Driver/linker-wrapper-hip-no-rdc.c (+24-8)
- (modified) clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp (+4-3)
``````````diff
diff --git a/clang/include/clang/Basic/TargetID.h b/clang/include/clang/Basic/TargetID.h
index cef9cb5f0fb21..902151d76556d 100644
--- a/clang/include/clang/Basic/TargetID.h
+++ b/clang/include/clang/Basic/TargetID.h
@@ -56,6 +56,11 @@ getConflictTargetIDCombination(const std::set<llvm::StringRef> &TargetIDs);
/// Check whether the provided target ID is compatible with the requested
/// target ID.
bool isCompatibleTargetID(llvm::StringRef Provided, llvm::StringRef Requested);
+
+/// Sanitize a target ID string for use in a file name.
+/// Replaces invalid characters (like ':') with safe characters (like '@').
+/// Currently only replaces ':' with '@' on Windows.
+std::string sanitizeTargetIDInFileName(llvm::StringRef TargetID);
} // namespace clang
#endif
diff --git a/clang/lib/Basic/TargetID.cpp b/clang/lib/Basic/TargetID.cpp
index 96e45ab5d9020..0aca490e17903 100644
--- a/clang/lib/Basic/TargetID.cpp
+++ b/clang/lib/Basic/TargetID.cpp
@@ -9,10 +9,13 @@
#include "clang/Basic/TargetID.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/Path.h"
#include "llvm/TargetParser/TargetParser.h"
#include "llvm/TargetParser/Triple.h"
#include <map>
#include <optional>
+#include <string>
namespace clang {
@@ -183,4 +186,11 @@ bool isCompatibleTargetID(llvm::StringRef Provided, llvm::StringRef Requested) {
return true;
}
+std::string sanitizeTargetIDInFileName(llvm::StringRef TargetID) {
+ std::string FileName = TargetID.str();
+ if (llvm::sys::path::is_style_windows(llvm::sys::path::Style::native))
+ llvm::replace(FileName, ':', '@');
+ return FileName;
+}
+
} // namespace clang
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 426fc796ffc20..de8d4601210ae 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -6285,12 +6285,7 @@ const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA,
StringRef OrigBoundArch, bool AtTopLevel,
bool MultipleArchs,
StringRef OffloadingPrefix) const {
- std::string BoundArch = OrigBoundArch.str();
- if (is_style_windows(llvm::sys::path::Style::native)) {
- // BoundArch may contains ':', which is invalid in file names on Windows,
- // therefore replace it with '%'.
- llvm::replace(BoundArch, ':', '@');
- }
+ std::string BoundArch = sanitizeTargetIDInFileName(OrigBoundArch);
llvm::PrettyStackTraceString CrashInfo("Computing output path");
// Output to a user requested destination?
diff --git a/clang/test/Driver/linker-wrapper-hip-no-rdc.c b/clang/test/Driver/linker-wrapper-hip-no-rdc.c
index 7545205f22ea0..addbec5fae613 100644
--- a/clang/test/Driver/linker-wrapper-hip-no-rdc.c
+++ b/clang/test/Driver/linker-wrapper-hip-no-rdc.c
@@ -1,4 +1,3 @@
-// UNSUPPORTED: system-windows
// REQUIRES: amdgpu-registered-target
// REQUIRES: lld
@@ -11,14 +10,30 @@ __attribute__((visibility("protected"), used)) int x;
// Create device binaries and package them
// RUN: %clang -cc1 %s -triple amdgcn-amd-amdhsa -emit-llvm-bc -o %t.amdgpu.bc
// RUN: llvm-offload-binary -o %t.out \
-// RUN: --image=file=%t.amdgpu.bc,kind=hip,triple=amdgcn-amd-amdhsa,arch=gfx1100 \
+// RUN: --image=file=%t.amdgpu.bc,kind=hip,triple=amdgcn-amd-amdhsa,arch=gfx9-4-generic:xnack+ \
// RUN: --image=file=%t.amdgpu.bc,kind=hip,triple=amdgcn-amd-amdhsa,arch=gfx1200
// Test that linker wrapper outputs .hipfb file without -r option for HIP non-RDC
// The linker wrapper is called directly with the packaged device binary (not embedded in host object)
// Note: When called directly (not through the driver), the linker wrapper processes architectures
// from the packaged binary. The test verifies it can process at least one architecture correctly.
-// RUN: clang-linker-wrapper --emit-fatbin-only --linker-path=/usr/bin/ld %t.out -o %t.hipfb 2>&1
+// RUN: %if system-windows %{ \
+// RUN: clang-linker-wrapper --wrapper-verbose --device-linker=amdgcn-amd-amdhsa=-v --device-compiler=-v --emit-fatbin-only --linker-path=/usr/bin/ld %t.out -o %t.hipfb 2>&1 | FileCheck %s --check-prefix=CMD-WIN \
+// RUN: %} %else %{ \
+// RUN: clang-linker-wrapper --wrapper-verbose --device-linker=amdgcn-amd-amdhsa=-v --device-compiler=-v --emit-fatbin-only --linker-path=/usr/bin/ld %t.out -o %t.hipfb 2>&1 | FileCheck %s --check-prefix=CMD-LINUX \
+// RUN: %}
+
+// On Linux, ':' is preserved in file names
+// CMD-LINUX-DAG: clang{{.*}} -o {{.*}}hipfb.amdgcn.gfx9-4-generic:xnack+{{.*}}.img
+// CMD-LINUX-DAG: clang{{.*}} -o {{.*}}hipfb.amdgcn.gfx1200{{.*}}.img
+// CMD-LINUX-DAG: ld.lld{{.*}} -o {{.*}}hipfb.amdgcn.gfx9-4-generic:xnack+{{.*}}.img
+// CMD-LINUX-DAG: ld.lld{{.*}} -o {{.*}}hipfb.amdgcn.gfx1200{{.*}}.img
+
+// On Windows, ':' is replaced with '@' in file names
+// CMD-WIN-DAG: clang{{.*}} -o {{.*}}hipfb.amdgcn.gfx9-4-generic at xnack+{{.*}}.img
+// CMD-WIN-DAG: clang{{.*}} -o {{.*}}hipfb.amdgcn.gfx1200{{.*}}.img
+// CMD-WIN-DAG: ld.lld{{.*}} -o {{.*}}hipfb.amdgcn.gfx9-4-generic at xnack+{{.*}}.img
+// CMD-WIN-DAG: ld.lld{{.*}} -o {{.*}}hipfb.amdgcn.gfx1200{{.*}}.img
// Verify the fat binary was created
// RUN: test -f %t.hipfb
@@ -26,16 +41,17 @@ __attribute__((visibility("protected"), used)) int x;
// List code objects in the fat binary
// RUN: clang-offload-bundler -type=o -input=%t.hipfb -list | FileCheck %s --check-prefix=HIP-FATBIN-LIST
-// HIP-FATBIN-LIST-DAG: hip-amdgcn-amd-amdhsa--gfx1100
+// HIP-FATBIN-LIST-DAG: hip-amdgcn-amd-amdhsa--gfx9-4-generic:xnack+
// HIP-FATBIN-LIST-DAG: hip-amdgcn-amd-amdhsa--gfx1200
// HIP-FATBIN-LIST-DAG: host-x86_64-unknown-linux-gnu-
// Extract code objects for both architectures from the fat binary
-// RUN: clang-offload-bundler -type=o -targets=hip-amdgcn-amd-amdhsa--gfx1100,hip-amdgcn-amd-amdhsa--gfx1200 \
-// RUN: -output=%t.gfx1100.co -output=%t.gfx1200.co -input=%t.hipfb -unbundle
+// Use '-' instead of ':' in file names to avoid issues on Windows
+// RUN: clang-offload-bundler -type=o -targets=hip-amdgcn-amd-amdhsa--gfx9-4-generic:xnack+,hip-amdgcn-amd-amdhsa--gfx1200 \
+// RUN: -output=%t.gfx9-4-generic-xnack+.co -output=%t.gfx1200.co -input=%t.hipfb -unbundle
// Verify extracted code objects exist and are not empty
-// RUN: test -f %t.gfx1100.co
-// RUN: test -s %t.gfx1100.co
+// RUN: test -f %t.gfx9-4-generic-xnack+.co
+// RUN: test -s %t.gfx9-4-generic-xnack+.co
// RUN: test -f %t.gfx1200.co
// RUN: test -s %t.gfx1200.co
diff --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index 4a4a43db6ef25..fcb6c591ec5ca 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -228,11 +228,13 @@ std::string getMainExecutable(const char *Name) {
Expected<StringRef> createOutputFile(const Twine &Prefix, StringRef Extension) {
std::scoped_lock<decltype(TempFilesMutex)> Lock(TempFilesMutex);
SmallString<128> OutputFile;
+ std::string PrefixStr = clang::sanitizeTargetIDInFileName(Prefix.str());
+
if (SaveTemps) {
- (Prefix + "." + Extension).toNullTerminatedStringRef(OutputFile);
+ (PrefixStr + "." + Extension).toNullTerminatedStringRef(OutputFile);
} else {
if (std::error_code EC =
- sys::fs::createTemporaryFile(Prefix, Extension, OutputFile))
+ sys::fs::createTemporaryFile(PrefixStr, Extension, OutputFile))
return createFileError(OutputFile, EC);
}
@@ -625,7 +627,6 @@ Expected<StringRef> writeOffloadFile(const OffloadFile &File) {
SmallString<128> Filename;
(Prefix + "-" + Binary.getTriple() + "-" + Binary.getArch())
.toVector(Filename);
- llvm::replace(Filename, ':', '-');
auto TempFileOrErr = createOutputFile(Filename, "o");
if (!TempFileOrErr)
return TempFileOrErr.takeError();
``````````
</details>
https://github.com/llvm/llvm-project/pull/168744
More information about the cfe-commits
mailing list