Skip to content

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Mar 7, 2025

So far the bot has been spewing "invalid argument" on windows,
and I'm guessing at why.

@arsenm arsenm added backend:AMDGPU clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Mar 7, 2025 — with Graphite App
@arsenm arsenm requested review from AlexVlx and yxsamliu March 7, 2025 05:43
@arsenm arsenm marked this pull request as ready for review March 7, 2025 05:43
@llvmbot
Copy link
Member

llvmbot commented Mar 7, 2025

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

So far the bot has been spewing "invalid argument" on windows,
and I'm guessing at why.


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

1 Files Affected:

  • (modified) clang/test/Driver/linker-wrapper.c (+4-5)
diff --git a/clang/test/Driver/linker-wrapper.c b/clang/test/Driver/linker-wrapper.c
index 0aa8f7d72432a..000ebee109f54 100644
--- a/clang/test/Driver/linker-wrapper.c
+++ b/clang/test/Driver/linker-wrapper.c
@@ -1,4 +1,3 @@
-// UNSUPPORTED: system-windows
 // REQUIRES: x86-registered-target
 // REQUIRES: nvptx-registered-target
 // REQUIRES: amdgpu-registered-target
@@ -119,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 \
@@ -170,8 +169,8 @@ __attribute__((visibility("protected"), used)) int x;
 // RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
 // RUN:   --linker-path=/usr/bin/ld %t-on.o %t-off.o %t.a -o a.out 2>&1 | FileCheck %s --check-prefix=AMD-TARGET-ID
 
-// AMD-TARGET-ID: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx90a:xnack+ -Wl,--no-undefined {{.*}}.o {{.*}}.o
-// AMD-TARGET-ID: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx90a:xnack- -Wl,--no-undefined {{.*}}.o {{.*}}.o
+// AMD-TARGET-ID: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu='gfx90a:xnack+' -Wl,--no-undefined {{.*}}.o {{.*}}.o
+// AMD-TARGET-ID: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu='gfx90a:xnack-' -Wl,--no-undefined {{.*}}.o {{.*}}.o
 
 // RUN: clang-offload-packager -o %t-lib.out \
 // RUN:   --image=file=%t.elf.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=generic
@@ -210,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
 

@arsenm arsenm force-pushed the users/arsenm/clang-driver-test-switch-requires-linux-to-unsupported-windows branch 3 times, most recently from cc6cbdd to 8e55948 Compare March 7, 2025 09:30
Base automatically changed from users/arsenm/clang-driver-test-switch-requires-linux-to-unsupported-windows to main March 7, 2025 09:31
So far the bot has been spewing "invalid argument" on windows,
and I'm guessing at why.
@arsenm arsenm force-pushed the users/arsenm/clang/enable-linker-wrapper-test-on-windows branch from 3f40104 to 6b912b4 Compare March 7, 2025 09:33
@jhuber6 jhuber6 requested a review from Meinersbur March 7, 2025 12:24
@jhuber6
Copy link
Contributor

jhuber6 commented Mar 7, 2025

@Meinersbur Do you have a good guess why this fails on Windows? It's difficult to triage without a windows machine.

@Meinersbur
Copy link
Member

Pre-merge on Linux fails too, but different symptoms. Let me try on my machine.

@Meinersbur
Copy link
Member

Meinersbur commented Mar 7, 2025

clang-offload-wrapper tries to write a file linker-wrapper.c.tmp-on-amdgcn-amd-amdhsa-gfx90a:xnack+-0c971c.o in the temp directory. : is not a valid character in filenames on Windows. The colon comes from the Binary.getArch() call here:

Prefix + "-" + Binary.getTriple() + "-" + Binary.getArch(), "o");

The error description propagation starting in TempFile::keep could be made a lot better if it included the filename.

@jhuber6
Copy link
Contributor

jhuber6 commented Mar 7, 2025

#130285 I'll see if this makes CI happy, thanks so much for your diagnosis.

@arsenm
Copy link
Contributor Author

arsenm commented Mar 7, 2025

Replaced by #130285

@arsenm arsenm closed this Mar 7, 2025
@arsenm arsenm deleted the users/arsenm/clang/enable-linker-wrapper-test-on-windows branch March 7, 2025 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AMDGPU 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