Skip to content

[SYCL] Fix handling of comma separated arch value when calling clang-offload-packager #20130

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: sycl
Choose a base branch
from
Open

[SYCL] Fix handling of comma separated arch value when calling clang-offload-packager #20130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9329697
[SYCL] Fix handling of comma separated arch value when calling clang-…
jzc Sep 18, 2025
3c38506
Address review comments
jzc Oct 2, 2025
1945480
Merge remote-tracking branch 'intel/sycl' into comma-tmp
jzc Oct 2, 2025
c8c9cb6
Add -Xsycl-target-linker to test
jzc Oct 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions clang/lib/Driver/ToolChains/Clang.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10325,6 +10325,7 @@ void OffloadPackager::ConstructJob(Compilation &C, const JobAction &JA,
File = C.getArgs().MakeArgString("@" + File);

StringRef Arch;
std::string TransformedArch;
if (OffloadAction->getOffloadingArch()) {
if (TC->getTripleString() == "spir64_gen-unknown-unknown") {
Arch = mapIntelGPUArchName(OffloadAction->getOffloadingArch());
Expand Down Expand Up @@ -10354,6 +10355,16 @@ void OffloadPackager::ConstructJob(Compilation &C, const JobAction &JA,
"kind=" + Kind.str(),
};

// When compiling like -fsycl-targets=spir64_gen -Xsycl-target-backend
// "-device pvc,bdw", the offloading arch will be "pvc,bdw", which
// contains a comma. We need to transform it to "arch=pvc,arch=bdw" when
// passing to clang-offload-packager.
SmallVector<StringRef> Archs;
Arch.split(Archs, ',');
if (Archs.size() > 1) {
Parts[2] = "arch=" + llvm::join(Archs, ",arch=");
}

if (TC->getDriver().isUsingOffloadLTO())
for (StringRef Feature : FeatureArgs)
Parts.emplace_back("feature=" + Feature.str());
Expand All @@ -10377,7 +10388,9 @@ void OffloadPackager::ConstructJob(Compilation &C, const JobAction &JA,
AL += " ";
AL += A;
}
Parts.emplace_back(C.getArgs().MakeArgString(Twine(Opt) + AL));
for (StringRef Split : llvm::split(AL, ',')) {
Parts.emplace_back(C.getArgs().MakeArgString(Twine(Opt) + Split));
}
};
const ArgList &Args =
C.getArgsForToolChain(nullptr, StringRef(), Action::OFK_SYCL);
Expand All @@ -10386,10 +10399,10 @@ void OffloadPackager::ConstructJob(Compilation &C, const JobAction &JA,
static_cast<const toolchains::SYCLToolChain &>(*TC);
SYCLTC.AddImpliedTargetArgs(TC->getTriple(), Args, BuildArgs, JA, *HostTC,
Arch);
SYCLTC.TranslateBackendTargetArgs(TC->getTriple(), Args, BuildArgs, Arch);
SYCLTC.TranslateBackendTargetArgs(TC->getTriple(), Args, BuildArgs);
createArgString("compile-opts=");
BuildArgs.clear();
SYCLTC.TranslateLinkerTargetArgs(TC->getTriple(), Args, BuildArgs, Arch);
SYCLTC.TranslateLinkerTargetArgs(TC->getTriple(), Args, BuildArgs);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the Arch arg here because it does not add the arguments when using -Xsycl-target-backend=spir64_gen '-device pvc -FOO' -Xsycl-target-linker=spir64_gen -LFOO correctly; it would only add them in a case where the =spir64_gen was removed or it was using the intel_gpu_pvc syntax.

Copy link
Contributor

Choose a reason for hiding this comment

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

By arguments, are you referring to the values '-device pvc -FOO' and -LFOO, getting passed to the target backend and linker, respectively?

createArgString("link-opts=");
}

Expand Down
8 changes: 7 additions & 1 deletion clang/test/Driver/sycl-ftarget-compile-fast.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,14 @@
// RUN: -fsycl-targets=spir64_gen -ftarget-compile-fast %s 2>&1 \
// RUN: | FileCheck -check-prefix=TARGET_COMPILE_FAST_GEN %s


// Due to how clang-offload-packager works, if we want a value in a key-value pair to
// have a comma, we need to specify the key twice, once for each part of the value
// separated by the comma. clang-offload-packager will then combine the two parts into a single
// value with a comma in between.
// TARGET_COMPILE_FAST_GEN: clang-offload-packager
// TARGET_COMPILE_FAST_GEN: compile-opts={{.*}}-options -igc_opts 'PartitionUnit=1,SubroutineThreshold=50000'
// TARGET_COMPILE_FAST_GEN: compile-opts={{.*}}-options -igc_opts 'PartitionUnit=1
// TARGET_COMPILE_FAST_GEN-SAME: compile-opts=SubroutineThreshold=50000'

// RUN: %clang -### -target x86_64-unknown-linux-gnu -fsycl --offload-new-driver \
// RUN: -ftarget-compile-fast %s 2>&1 \
Expand Down
31 changes: 30 additions & 1 deletion clang/test/Driver/sycl-offload-new-driver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@
// RUN: %clangxx -fsycl -### -fsycl-targets=spir64_gen --offload-new-driver \
// RUN: -Xsycl-target-backend=spir64_gen "-device pvc,bdw" %s 2>&1 \
// RUN: | FileCheck -check-prefix COMMA_FILE %s
// COMMA_FILE: clang-offload-packager{{.*}} "--image=file={{.*}}pvc@bdw{{.*}},triple=spir64_gen-unknown-unknown,arch=pvc,bdw,kind=sycl,compile-opts=-device_options pvc -ze-intel-enable-auto-large-GRF-mode"
// COMMA_FILE: clang-offload-packager{{.*}} "--image=file={{.*}}pvc@bdw{{.*}},triple=spir64_gen-unknown-unknown,arch=pvc,arch=bdw,kind=sycl,compile-opts=-device_options pvc -ze-intel-enable-auto-large-GRF-mode -device pvc,compile-opts=bdw"

/// Verify the arch value for the packager is populated with different
/// scenarios for spir64_gen
Expand All @@ -211,6 +211,35 @@
// RUN: | FileCheck -check-prefix ARCH_CHECK %s
// ARCH_CHECK: clang-offload-packager{{.*}} "--image=file={{.*}}triple=spir64_gen-unknown-unknown,arch=bdw,kind=sycl{{.*}}"

// Verify when a comma-separated list of architectures is provided in -device, they are
// passed to clang-offload-packager correctly
// RUN: %clangxx -fsycl -### -fsycl-targets=spir64_gen --offload-new-driver \
// RUN: -Xsycl-target-backend "-device pvc,bdw" %s 2>&1 \
Copy link
Contributor

Choose a reason for hiding this comment

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

Another suggestion is to document this behavior in the help text or documentation instead of adding diagnostic in the code.

// RUN: | FileCheck -check-prefix MULTI_ARCH %s
// MULTI_ARCH: clang-offload-packager{{.*}} "--image=file={{.*}}triple=spir64_gen-unknown-unknown,arch=pvc,arch=bdw,kind=sycl
// MULTI_ARCH-SAME: compile-opts=-device_options pvc -ze-intel-enable-auto-large-GRF-mode -device pvc,compile-opts=bdw"

// RUN: %clangxx -fsycl -### --offload-new-driver \
// RUN: -fsycl-targets=nvptx64-nvidia-cuda,amdgcn-amd-amdhsa,spir64_gen \
// RUN: -Xsycl-target-backend=amdgcn-amd-amdhsa --offload-arch=gfx908,gfx1010 \
// RUN: -Xsycl-target-backend=nvptx64-nvidia-cuda --offload-arch=sm_86,sm_87,sm_89 \
// RUN: -Xsycl-target-backend=spir64_gen "-device pvc,bdw" \
// RUN: -Xsycl-target-linker=spir64_gen "-DFOO,BAR" \
// RUN: -nogpulib %s 2>&1 | FileCheck -check-prefix=MULTI_ARCH2 %s
// MULTI_ARCH2: clang-offload-packager{{.*}} "--image=file={{.*}}triple=amdgcn-amd-amdhsa,arch=gfx1010,kind=sycl,compile-opts=--offload-arch=gfx908,compile-opts=gfx1010"
// MULTI_ARCH2-SAME: "--image=file={{.*}}triple=amdgcn-amd-amdhsa,arch=gfx908,kind=sycl,compile-opts=--offload-arch=gfx908,compile-opts=gfx1010"
// MULTI_ARCH2-SAME: "--image=file={{.*}}triple=nvptx64-nvidia-cuda,arch=sm_86,kind=sycl,compile-opts=--offload-arch=sm_86,compile-opts=sm_87,compile-opts=sm_89"
// MULTI_ARCH2-SAME: "--image=file={{.*}}triple=nvptx64-nvidia-cuda,arch=sm_87,kind=sycl,compile-opts=--offload-arch=sm_86,compile-opts=sm_87,compile-opts=sm_89"
// MULTI_ARCH2-SAME: "--image=file={{.*}}triple=nvptx64-nvidia-cuda,arch=sm_89,kind=sycl,compile-opts=--offload-arch=sm_86,compile-opts=sm_87,compile-opts=sm_89"
// MULTI_ARCH2-SAME: "--image=file={{.*}}triple=spir64_gen-unknown-unknown,arch=pvc,arch=bdw,kind=sycl,compile-opts=-device_options pvc -ze-intel-enable-auto-large-GRF-mode -device pvc,compile-opts=bdw,link-opts=-DFOO,link-opts=BAR"

// Verify that the driver correctly handles link-opt and compile-opt values with commas
// RUN: %clangxx -fsycl -### -fsycl-targets=spir64_gen --offload-new-driver \
// RUN: -Xsycl-target-backend "-device bdw -FOO a,b" \
// RUN: -Xsycl-target-linker "-BAR x,y" %s 2>&1 \
// RUN: | FileCheck -check-prefix COMMA_OPTS %s
// COMMA_OPTS: clang-offload-packager{{.*}} "--image=file={{.*}}triple=spir64_gen-unknown-unknown,arch=bdw,kind=sycl,compile-opts=-device bdw -FOO a,compile-opts=b,link-opts=-BAR x,link-opts=y"

/// Verify that --cuda-path is passed to clang-linker-wrapper for SYCL offload
// RUN: %clangxx -fsycl -### -fsycl-targets=nvptx64-nvidia-cuda -fno-sycl-libspirv \
// RUN: --cuda-gpu-arch=sm_20 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda %s \
Expand Down
Loading