-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[LinkerWrapper] Try to fix testing on Windows #130285
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
Conversation
Summary: Thanks to @Meinersbur for finding this. The `:` character used in AMD's target-id's is invalid on windows. This patch replaces them with `_`.
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Joseph Huber (jhuber6) ChangesSummary: Full diff: https://github.com/llvm/llvm-project/pull/130285.diff 2 Files Affected:
diff --git a/clang/test/Driver/linker-wrapper.c b/clang/test/Driver/linker-wrapper.c
index 7586b87743bf5..1f7a065fec0dc 100644
--- a/clang/test/Driver/linker-wrapper.c
+++ b/clang/test/Driver/linker-wrapper.c
@@ -2,8 +2,6 @@
// REQUIRES: nvptx-registered-target
// REQUIRES: amdgpu-registered-target
-// REQUIRES: system-linux
-
// An externally visible variable so static libraries extract.
__attribute__((visibility("protected"), used)) int x;
@@ -120,7 +118,7 @@ __attribute__((visibility("protected"), used)) int x;
// HIP: clang{{.*}} -o [[IMG_GFX90A:.+]] --target=amdgcn-amd-amdhsa -mcpu=gfx90a
// HIP: clang{{.*}} -o [[IMG_GFX908:.+]] --target=amdgcn-amd-amdhsa -mcpu=gfx908
-// HIP: clang-offload-bundler{{.*}}-type=o -bundle-align=4096 -compress -compression-level=6 -targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa--gfx90a,hip-amdgcn-amd-amdhsa--gfx908 -input=/dev/null -input=[[IMG_GFX90A]] -input=[[IMG_GFX908]] -output={{.*}}.hipfb
+// HIP: clang-offload-bundler{{.*}}-type=o -bundle-align=4096 -compress -compression-level=6 -targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa--gfx90a,hip-amdgcn-amd-amdhsa--gfx908 -input={{/dev/null|NUL}} -input=[[IMG_GFX90A]] -input=[[IMG_GFX908]] -output={{.*}}.hipfb
// RUN: clang-offload-packager -o %t.out \
// RUN: --image=file=%t.elf.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx908 \
@@ -211,7 +209,7 @@ __attribute__((visibility("protected"), used)) int x;
// RUN: %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=RELOCATABLE-LINK-HIP
// RELOCATABLE-LINK-HIP: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa
-// RELOCATABLE-LINK-HIP: clang-offload-bundler{{.*}} -type=o -bundle-align=4096 -targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa--gfx90a -input=/dev/null -input={{.*}} -output={{.*}}
+// RELOCATABLE-LINK-HIP: clang-offload-bundler{{.*}} -type=o -bundle-align=4096 -targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa--gfx90a -input={{/dev/null|NUL}} -input={{.*}} -output={{.*}}
// RELOCATABLE-LINK-HIP: /usr/bin/ld.lld{{.*}}-r
// RELOCATABLE-LINK-HIP: llvm-objcopy{{.*}}a.out --remove-section .llvm.offloading
diff --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index 10dfb008cbec2..f111a9cadac6a 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -599,9 +599,10 @@ Expected<StringRef> writeOffloadFile(const OffloadFile &File) {
StringRef Prefix =
sys::path::stem(Binary.getMemoryBufferRef().getBufferIdentifier());
-
- auto TempFileOrErr = createOutputFile(
- Prefix + "-" + Binary.getTriple() + "-" + Binary.getArch(), "o");
+ std::string Filename =
+ (Prefix + "-" + Binary.getTriple() + "-" + Binary.getArch()).str();
+ llvm::replace(Filename, ':', '-');
+ auto TempFileOrErr = createOutputFile(Filename, "o");
if (!TempFileOrErr)
return TempFileOrErr.takeError();
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not aware of any filename sanitizer utility either.
Co-authored-by: Michael Kruse <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary: Thanks to @Meinersbur for finding this. The `:` character used in AMD's target-id's is invalid on windows. This patch replaces them with `-`. --------- Co-authored-by: Michael Kruse <[email protected]>
Summary:
Thanks to @Meinersbur for finding this. The
:
character used in AMD'starget-id's is invalid on windows. This patch replaces them with
-
.