From 3339a779ff2658a6d870797c3abb0175c6b60680 Mon Sep 17 00:00:00 2001 From: Gergely Meszaros Date: Tue, 24 Jun 2025 06:22:55 -0700 Subject: [PATCH 1/3] [SYCL][Driver] Base libspirv path on sycl install candidates ... instead of the resource directory. This matches the behaviour of SYCL device libraries. The resource directory has nothing to do with libspirv, and it might be changed by the `-resource-dir` option, which would lead us to not find it. --- clang/lib/Driver/ToolChains/SYCL.cpp | 21 +++++++++---------- clang/test/Driver/Inputs/SYCL/bin/.gitkeep | 0 clang/test/Driver/sycl-libspirv-toolchain.cpp | 18 ++++++++-------- 3 files changed, 19 insertions(+), 20 deletions(-) create mode 100644 clang/test/Driver/Inputs/SYCL/bin/.gitkeep diff --git a/clang/lib/Driver/ToolChains/SYCL.cpp b/clang/lib/Driver/ToolChains/SYCL.cpp index 164a4c297c369..70c91502654cb 100644 --- a/clang/lib/Driver/ToolChains/SYCL.cpp +++ b/clang/lib/Driver/ToolChains/SYCL.cpp @@ -182,11 +182,9 @@ const char *SYCLInstallationDetector::findLibspirvPath( const SmallString<64> Basename = getLibSpirvBasename(DeviceTriple, HostTriple); auto searchAt = [&](StringRef Path, const Twine &a = "", const Twine &b = "", - const Twine &c = "", const Twine &d = "", - const Twine &e = "") -> const char * { + const Twine &c = "") -> const char * { SmallString<128> LibraryPath(Path); - llvm::sys::path::append(LibraryPath, a, b, c, d); - llvm::sys::path::append(LibraryPath, e, Basename); + llvm::sys::path::append(LibraryPath, a, b, c, Basename); if (Args.hasArgNoClaim(options::OPT__HASH_HASH_HASH) || llvm::sys::fs::exists(LibraryPath)) @@ -195,14 +193,15 @@ const char *SYCLInstallationDetector::findLibspirvPath( return nullptr; }; - // Otherwise, assume libclc is installed at the same prefix as clang - // Expected path w/out install. - if (const char *R = searchAt(D.ResourceDir, "..", "..", "clc")) - return R; + for (const auto &IC : InstallationCandidates) { + // Expected path w/out install. + if (const char *R = searchAt(IC, "lib", "clc")) + return R; - // Expected path w/ install. - if (const char *R = searchAt(D.ResourceDir, "..", "..", "..", "share", "clc")) - return R; + // Expected path w/ install. + if (const char *R = searchAt(IC, "share", "clc")) + return R; + } return nullptr; } diff --git a/clang/test/Driver/Inputs/SYCL/bin/.gitkeep b/clang/test/Driver/Inputs/SYCL/bin/.gitkeep new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/clang/test/Driver/sycl-libspirv-toolchain.cpp b/clang/test/Driver/sycl-libspirv-toolchain.cpp index 1dc2ddf02cbe5..5587edfe3bbb7 100644 --- a/clang/test/Driver/sycl-libspirv-toolchain.cpp +++ b/clang/test/Driver/sycl-libspirv-toolchain.cpp @@ -15,7 +15,7 @@ // RUN: | FileCheck %s --check-prefixes=CHECK-LINUX // CHECK-LINUX: "-cc1"{{.*}} "-fsycl-is-device"{{.*}} "-mlink-builtin-bitcode" "{{.*[\\/]}}remangled-l64-signed_char.libspirv-nvptx64-nvidia-cuda.bc" // -// RUN: %clang -### -resource-dir %{resource_dir} -fsycl -fsycl-targets=amdgcn-amd-amdhsa -Xsycl-target-backend --offload-arch=gfx908 -nogpulib -target x86_64-unknown-windows-msvc %s 2>&1 \ +// RUN: %clang -### -ccc-install-dir %{install_dir} -fsycl -fsycl-targets=amdgcn-amd-amdhsa -Xsycl-target-backend --offload-arch=gfx908 -nogpulib -target x86_64-unknown-windows-msvc %s 2>&1 \ // RUN: | FileCheck %s --check-prefixes=CHECK-AMDGCN-WINDOWS // CHECK-AMDGCN-WINDOWS: "-cc1"{{.*}} "-fsycl-is-device"{{.*}} "-mlink-builtin-bitcode" "{{.*[\\/]}}remangled-l32-signed_char.libspirv-amdgcn-amd-amdhsa.bc" // @@ -30,22 +30,22 @@ // CHECK-CUDA: "-cc1"{{.*}} "-fcuda-is-device" // CHECK-CUDA-NOT: "-mlink-builtin-bitcode" "{{.*}}.libspirv-{{.*}}.bc" // -// The path to the remangled libspirv bitcode file is determined by the resource directory. +// The path to the remangled libspirv bitcode file is determined by the installation directory // RUN: %clang -### -ccc-install-dir %{install_dir} -resource-dir %{resource_dir} -fsycl -fsycl-targets=nvptx64-nvidia-cuda -nocudalib %s 2>&1 \ // RUN: | FileCheck %s -DINSTALL_DIR=%{install_dir} -DRESOURCE_DIR=%{resource_dir} --check-prefixes=CHECK-DIR -// CHECK-DIR: "-cc1"{{.*}} "-fsycl-is-device"{{.*}} "-mlink-builtin-bitcode" "[[RESOURCE_DIR]]{{.*[\\/]}}remangled-{{.*}}.libspirv-nvptx64-nvidia-cuda.bc" +// CHECK-DIR: "-cc1"{{.*}} "-fsycl-is-device"{{.*}} "-mlink-builtin-bitcode" "[[INSTALL_DIR]]{{.*[\\/]}}remangled-{{.*}}.libspirv-nvptx64-nvidia-cuda.bc" // // The `-###` option disables file existence checks // DEFINE: %{nonexistent_dir} = %/S/Inputs/SYCL/does_not_exist/lib/clang/resource_dir -// RUN: %clang -### -resource-dir %{nonexistent_dir} -fsycl -fsycl-targets=nvptx64-nvidia-cuda -nocudalib %s 2>&1 \ -// RUN: | FileCheck %s -DDIR=%{nonexistent_dir} --check-prefixes=CHECK-HHH-NONEXISTENT +// RUN: %clang -### -ccc-install-dir %{nonexistent_dir} -fsycl -fsycl-targets=nvptx64-nvidia-cuda -nocudalib %s 2>&1 \ +// RUN: | FileCheck %s -DDIR=%{nonexistent_dir} --check-prefixes=CHECK-HHH-NONEXISTENT // CHECK-HHH-NONEXISTENT: "-cc1"{{.*}} "-fsycl-is-device"{{.*}} "-mlink-builtin-bitcode" "[[DIR]]{{.*[\\/]}}remangled-{{.*}}.libspirv-nvptx64-nvidia-cuda.bc" // -// RUN: %clang -### -resource-dir %{nonexistent_dir} -fsycl -fsycl-targets=amdgcn-amd-amdhsa -Xsycl-target-backend --offload-arch=gfx908 -nogpulib %s 2>&1 \ -// RUN: | FileCheck %s -DDIR=%{nonexistent_dir} --check-prefixes=CHECK-AMDGCN-HHH-NONEXISTENT +// RUN: %clang -### -ccc-install-dir %{nonexistent_dir} -fsycl -fsycl-targets=amdgcn-amd-amdhsa -Xsycl-target-backend --offload-arch=gfx908 -nogpulib %s 2>&1 \ +// RUN: | FileCheck %s -DDIR=%{nonexistent_dir} --check-prefixes=CHECK-AMDGCN-HHH-NONEXISTENT // CHECK-AMDGCN-HHH-NONEXISTENT: "-cc1"{{.*}} "-fsycl-is-device"{{.*}} "-mlink-builtin-bitcode" "[[DIR]]{{.*[\\/]}}remangled-{{.*}}.libspirv-amdgcn-amd-amdhsa.bc" // // `-fdriver-only` has no such special handling, so it will not find the file -// RUN: not %clang -fdriver-only -resource-dir %{nonexistent_dir} -fsycl -fsycl-targets=nvptx64-nvidia-cuda -nocudalib %s 2>&1 \ -// RUN: | FileCheck %s --check-prefixes=CHECK-DO-NONEXISTENT +// RUN: not %clang -fdriver-only -ccc-install-dir %{nonexistent_dir} -fsycl -fsycl-targets=nvptx64-nvidia-cuda -nocudalib %s 2>&1 \ +// RUN: | FileCheck %s -DDIR=%{nonexistent_dir} --check-prefixes=CHECK-DO-NONEXISTENT // CHECK-DO-NONEXISTENT: error: cannot find 'remangled-{{.*}}.libspirv-nvptx64-nvidia-cuda.bc'; provide path to libspirv library via '-fsycl-libspirv-path', or pass '-fno-sycl-libspirv' to build without linking with libspirv From 0bb5db5ab52fc3c93ed8cc22e39a4138a1bc1256 Mon Sep 17 00:00:00 2001 From: Gergely Meszaros Date: Tue, 24 Jun 2025 07:33:57 -0700 Subject: [PATCH 2/3] [SYCL][Driver] Emit unused argument warning for -fno-libspirv Previously the `-fno-libspirv` option was not warned about when there was no SYCL compilation. Remove the explicit target check for the targets that support `-fno-libspirv` and instead rely on each target to emit appropriate warnings when its used. This commit slightly degrades diagnostic quality, from "ignoring '-fno-sycl-libspirv' option as it is not currently supported for target" to "argument unused during compilation: '-fno-sycl-libspirv'" , but I believe this is acceptable as it allows to remove the list of targets that support the option from the driver. Additionally, now if the user mixes targets that support and do not support `-fno-libspirv` in the same compilation, they will not get warnings that are not actionable. --- clang/lib/Driver/Driver.cpp | 13 ------------- clang/lib/Driver/ToolChains/SYCL.cpp | 13 +++++++++++-- clang/test/Driver/sycl-fno-libspirv-warn.cpp | 4 ++-- clang/test/Driver/sycl-libspirv-toolchain.cpp | 4 ++-- 4 files changed, 15 insertions(+), 19 deletions(-) diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 530d1fd3e478b..ca6339d9cda97 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -1567,19 +1567,6 @@ void Driver::CreateOffloadingDeviceToolChains(Compilation &C, } } - // -fno-sycl-libspirv flag is reserved for very unusual cases where the - // libspirv library is not linked when using CUDA/HIP: so output appropriate - // warnings. - if (C.getInputArgs().hasArg(options::OPT_fno_sycl_libspirv)) { - for (auto &TT : UniqueSYCLTriplesVec) { - if (TT.isNVPTX() || TT.isAMDGCN()) { - Diag(diag::warn_flag_no_sycl_libspirv) << TT.getTriple(); - continue; - } - Diag(diag::warn_drv_unsupported_option_for_target) - << "-fno-sycl-libspirv" << TT.getTriple() << 0; - } - } // -fsycl-fp64-conv-emu is valid only for AOT compilation with an Intel GPU // target. For other scenarios, we emit a warning message. if (C.getInputArgs().hasArg(options::OPT_fsycl_fp64_conv_emu)) { diff --git a/clang/lib/Driver/ToolChains/SYCL.cpp b/clang/lib/Driver/ToolChains/SYCL.cpp index 70c91502654cb..511b349406951 100644 --- a/clang/lib/Driver/ToolChains/SYCL.cpp +++ b/clang/lib/Driver/ToolChains/SYCL.cpp @@ -209,9 +209,18 @@ const char *SYCLInstallationDetector::findLibspirvPath( void SYCLInstallationDetector::addLibspirvLinkArgs( const llvm::Triple &DeviceTriple, const llvm::opt::ArgList &DriverArgs, const llvm::Triple &HostTriple, llvm::opt::ArgStringList &CC1Args) const { - if (DriverArgs.hasArg(options::OPT_fno_sycl_libspirv) || - D.offloadDeviceOnly()) + DriverArgs.claimAllArgs(options::OPT_fno_sycl_libspirv); + + if (D.offloadDeviceOnly()) + return; + + if (DriverArgs.hasArg(options::OPT_fno_sycl_libspirv)) { + // -fno-sycl-libspirv flag is reserved for very unusual cases where the + // libspirv library is not linked when required by the device: so output appropriate + // warnings. + D.Diag(diag::warn_flag_no_sycl_libspirv) << DeviceTriple.str(); return; + } if (const char *LibSpirvFile = findLibspirvPath(DeviceTriple, DriverArgs, HostTriple)) { diff --git a/clang/test/Driver/sycl-fno-libspirv-warn.cpp b/clang/test/Driver/sycl-fno-libspirv-warn.cpp index 5b9307a80f26c..a6fe0e67279d3 100644 --- a/clang/test/Driver/sycl-fno-libspirv-warn.cpp +++ b/clang/test/Driver/sycl-fno-libspirv-warn.cpp @@ -1,7 +1,7 @@ /// Test that appropriate warnings are output when -fno-sycl-libspirv is used. -// RUN: not %clangxx -fsycl -fsycl-targets=nvptx64-nvidia-cuda,amdgcn-amd-amdhsa -fno-sycl-libspirv %s -### 2>&1 | FileCheck %s +// RUN: %clangxx -fsycl -fsycl-targets=nvptx64-nvidia-cuda,amdgcn-amd-amdhsa -Xsycl-target-backend=amdgcn-amd-amdhsa --offload-arch=gfx908 -nogpulib -fno-sycl-libspirv %s -### 2>&1 | FileCheck %s // CHECK-DAG: warning: '-fno-sycl-libspirv' should not be used with target 'nvptx64-nvidia-cuda'; libspirv is required for correct behavior [-Wunsafe-libspirv-not-linked] // CHECK-DAG: warning: '-fno-sycl-libspirv' should not be used with target 'amdgcn-amd-amdhsa'; libspirv is required for correct behavior [-Wunsafe-libspirv-not-linked] // RUN: %clangxx -fsycl -fsycl-targets=spir64-unknown-unknown -fno-sycl-libspirv %s -### 2>&1 | FileCheck --check-prefix=CHECK-SPIR64 %s -// CHECK-SPIR64: ignoring '-fno-sycl-libspirv' option as it is not currently supported for target 'spir64-unknown-unknown' [-Woption-ignored] +// CHECK-SPIR64: argument unused during compilation: '-fno-sycl-libspirv' [-Wunused-command-line-argument] diff --git a/clang/test/Driver/sycl-libspirv-toolchain.cpp b/clang/test/Driver/sycl-libspirv-toolchain.cpp index 5587edfe3bbb7..3c0888e795506 100644 --- a/clang/test/Driver/sycl-libspirv-toolchain.cpp +++ b/clang/test/Driver/sycl-libspirv-toolchain.cpp @@ -24,9 +24,9 @@ // CHECK-DEVICE-ONLY: "-cc1"{{.*}} "-fsycl-is-device" // CHECK-DEVICE-ONLY-NOT: "-mlink-builtin-bitcode" "{{.*}}.libspirv-{{.*}}.bc" // -// Only link libspirv in SYCL language mode, but `-fno-sycl-libspirv` does not result in a warning +// Only link libspirv in SYCL language mode, `-fno-sycl-libspirv` should result in a warning // RUN: %clang -### -x cu -fno-sycl-libspirv -nocudainc -nocudalib %s 2>&1 | FileCheck %s --check-prefixes=CHECK-CUDA -// CHECK-CUDA-NOT: warning: argument unused during compilation: '-fno-sycl-libspirv' [-Wunused-command-line-argument] +// CHECK-CUDA: warning: argument unused during compilation: '-fno-sycl-libspirv' [-Wunused-command-line-argument] // CHECK-CUDA: "-cc1"{{.*}} "-fcuda-is-device" // CHECK-CUDA-NOT: "-mlink-builtin-bitcode" "{{.*}}.libspirv-{{.*}}.bc" // From e405eb057cd85fea6a9709d85eadca7aedbb92b0 Mon Sep 17 00:00:00 2001 From: Gergely Meszaros Date: Tue, 24 Jun 2025 07:50:32 -0700 Subject: [PATCH 3/3] [SYCL][Driver][libdevice] Link libspirv in `-fsycl-device-only` mode Previously, the `-fsycl-device-only` mode disabled linking libspirv. `-fsycl-device-only` should produce the same LLVM-IR as `-fsycl` would, but without any host actions. Libdevice is the likely reason why this was done as is, but it should not be the default. This patch changes libdevice options to disable libspirv linking there, and enables linking by default in `-fsycl-device-only` mode. --- clang/lib/Driver/ToolChains/SYCL.cpp | 5 ----- clang/test/Driver/sycl-libspirv-toolchain.cpp | 3 +-- libdevice/cmake/modules/SYCLLibdevice.cmake | 4 ++-- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/clang/lib/Driver/ToolChains/SYCL.cpp b/clang/lib/Driver/ToolChains/SYCL.cpp index 511b349406951..25240e34a8df7 100644 --- a/clang/lib/Driver/ToolChains/SYCL.cpp +++ b/clang/lib/Driver/ToolChains/SYCL.cpp @@ -209,11 +209,6 @@ const char *SYCLInstallationDetector::findLibspirvPath( void SYCLInstallationDetector::addLibspirvLinkArgs( const llvm::Triple &DeviceTriple, const llvm::opt::ArgList &DriverArgs, const llvm::Triple &HostTriple, llvm::opt::ArgStringList &CC1Args) const { - DriverArgs.claimAllArgs(options::OPT_fno_sycl_libspirv); - - if (D.offloadDeviceOnly()) - return; - if (DriverArgs.hasArg(options::OPT_fno_sycl_libspirv)) { // -fno-sycl-libspirv flag is reserved for very unusual cases where the // libspirv library is not linked when required by the device: so output appropriate diff --git a/clang/test/Driver/sycl-libspirv-toolchain.cpp b/clang/test/Driver/sycl-libspirv-toolchain.cpp index 3c0888e795506..eac98ad9b17c2 100644 --- a/clang/test/Driver/sycl-libspirv-toolchain.cpp +++ b/clang/test/Driver/sycl-libspirv-toolchain.cpp @@ -21,8 +21,7 @@ // // RUN: %clang -### -fsycl -fsycl-device-only -fsycl-targets=nvptx64-nvidia-cuda -nocudalib %s 2>&1 \ // RUN: | FileCheck %s --check-prefixes=CHECK-DEVICE-ONLY -// CHECK-DEVICE-ONLY: "-cc1"{{.*}} "-fsycl-is-device" -// CHECK-DEVICE-ONLY-NOT: "-mlink-builtin-bitcode" "{{.*}}.libspirv-{{.*}}.bc" +// CHECK-DEVICE-ONLY: "-cc1"{{.*}} "-fsycl-is-device"{{.*}} "-mlink-builtin-bitcode" "{{.*[\\/]}}remangled-{{.*}}.libspirv-nvptx64-nvidia-cuda.bc" // // Only link libspirv in SYCL language mode, `-fno-sycl-libspirv` should result in a warning // RUN: %clang -### -x cu -fno-sycl-libspirv -nocudainc -nocudalib %s 2>&1 | FileCheck %s --check-prefixes=CHECK-CUDA diff --git a/libdevice/cmake/modules/SYCLLibdevice.cmake b/libdevice/cmake/modules/SYCLLibdevice.cmake index 9a56ffbb35c26..6f6bab7ebff53 100644 --- a/libdevice/cmake/modules/SYCLLibdevice.cmake +++ b/libdevice/cmake/modules/SYCLLibdevice.cmake @@ -82,13 +82,13 @@ set(devicelib_arch) if ("NVPTX" IN_LIST LLVM_TARGETS_TO_BUILD) list(APPEND devicelib_arch nvptx64-nvidia-cuda) set(compile_opts_nvptx64-nvidia-cuda "-fsycl-targets=nvptx64-nvidia-cuda" - "-Xsycl-target-backend" "--cuda-gpu-arch=sm_50" "-nocudalib") + "-Xsycl-target-backend" "--cuda-gpu-arch=sm_50" "-nocudalib" "-fno-sycl-libspirv" "-Wno-unsafe-libspirv-not-linked") set(opt_flags_nvptx64-nvidia-cuda "-O3" "--nvvm-reflect-enable=false") endif() if("AMDGPU" IN_LIST LLVM_TARGETS_TO_BUILD) list(APPEND devicelib_arch amdgcn-amd-amdhsa) set(compile_opts_amdgcn-amd-amdhsa "-nogpulib" "-fsycl-targets=amdgcn-amd-amdhsa" - "-Xsycl-target-backend" "--offload-arch=gfx942") + "-Xsycl-target-backend" "--offload-arch=gfx942" "-fno-sycl-libspirv" "-Wno-unsafe-libspirv-not-linked") set(opt_flags_amdgcn-amd-amdhsa "-O3" "--amdgpu-oclc-reflect-enable=false") endif()