From 37e87543483e268e033d1bfb6076ee2a3bfd0a70 Mon Sep 17 00:00:00 2001 From: "Gainullin, Artur" Date: Tue, 13 Feb 2024 10:23:45 -0800 Subject: [PATCH 1/2] [SYCL] Creating version-agnostic copy of the SYCL import library Ultimate goal is to get rid of most of the usages SYCL_MAJOR_VERSION in clang, so that we don't need to keep that version in sync with SYCL RT. Currently only one usage left which is related to windows-gnu triple which is something we don't test (using compiler in mingw environment) and that will need to be handled separately, as I wasn't able to succesfully build/experiment with that. Other reason why this is needed is because some users want to explicitely link with sycl library and they don't want to update version all the time. It is possible to just generated version-agnostic .lib files but I think we need to keep versioned files as well in case if somebody will have a scenario when multiple releases are in the LIB env variable and they want to link with specific version explicitely. --- clang/lib/Driver/CMakeLists.txt | 2 +- clang/lib/Driver/ToolChains/Clang.cpp | 15 +++----- clang/lib/Driver/ToolChains/MSVC.cpp | 9 ++--- sycl/source/CMakeLists.txt | 38 ++++++++++++------- .../windows_version_agnostic_sycl_lib.cpp | 16 ++++++++ 5 files changed, 52 insertions(+), 28 deletions(-) create mode 100644 sycl/test-e2e/Basic/windows_version_agnostic_sycl_lib.cpp diff --git a/clang/lib/Driver/CMakeLists.txt b/clang/lib/Driver/CMakeLists.txt index d5d2bd4aab75e..2949b2a506887 100644 --- a/clang/lib/Driver/CMakeLists.txt +++ b/clang/lib/Driver/CMakeLists.txt @@ -18,7 +18,7 @@ if(WIN32) endif() # This must be in sync with llvm/sycl/CMakeLists.txt. -SET_SOURCE_FILES_PROPERTIES( ToolChains/MSVC.cpp ToolChains/Clang.cpp +SET_SOURCE_FILES_PROPERTIES( ToolChains/Clang.cpp PROPERTIES COMPILE_DEFINITIONS SYCL_MAJOR_VERSION="7" ) add_clang_library(clangDriver diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 9cafc66bd2ea2..638b9bbcb2390 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -5035,16 +5035,14 @@ static void ProcessVSRuntimeLibrary(const ArgList &Args, !Args.hasArg(options::OPT_nolibsycl)) { if (RTOptionID == options::OPT__SLASH_MDd) { if (Args.hasArg(options::OPT_fpreview_breaking_changes)) - CmdArgs.push_back("--dependent-lib=sycl" SYCL_MAJOR_VERSION - "-previewd"); + CmdArgs.push_back("--dependent-lib=sycl-previewd"); else - CmdArgs.push_back("--dependent-lib=sycl" SYCL_MAJOR_VERSION "d"); + CmdArgs.push_back("--dependent-lib=sycld"); } else { if (Args.hasArg(options::OPT_fpreview_breaking_changes)) - CmdArgs.push_back("--dependent-lib=sycl" SYCL_MAJOR_VERSION - "-preview"); + CmdArgs.push_back("--dependent-lib=sycl-preview"); else - CmdArgs.push_back("--dependent-lib=sycl" SYCL_MAJOR_VERSION); + CmdArgs.push_back("--dependent-lib=sycl"); } CmdArgs.push_back("--dependent-lib=sycl-devicelib-host"); } @@ -6598,10 +6596,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, if (!D.IsCLMode() && TC.getTriple().isWindowsMSVCEnvironment()) { if (isDependentLibAdded(Args, "msvcrtd")) { if (Args.hasArg(options::OPT_fpreview_breaking_changes)) - CmdArgs.push_back("--dependent-lib=sycl" SYCL_MAJOR_VERSION - "-previewd"); + CmdArgs.push_back("--dependent-lib=sycl-previewd"); else - CmdArgs.push_back("--dependent-lib=sycl" SYCL_MAJOR_VERSION "d"); + CmdArgs.push_back("--dependent-lib=sycld"); } } else if (!D.IsCLMode() && TC.getTriple().isWindowsGNUEnvironment()) { if (Args.hasArg(options::OPT_fpreview_breaking_changes)) diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp index 5e08965320004..ea695abda3bfa 100644 --- a/clang/lib/Driver/ToolChains/MSVC.cpp +++ b/clang/lib/Driver/ToolChains/MSVC.cpp @@ -138,15 +138,14 @@ void visualstudio::Linker::ConstructJob(Compilation &C, const JobAction &JA, if (!Args.hasArg(options::OPT__SLASH_MDd) && !isDependentLibAdded(Args, "msvcrtd")) { if (Args.hasArg(options::OPT_fpreview_breaking_changes)) - CmdArgs.push_back("-defaultlib:sycl" SYCL_MAJOR_VERSION "-preview.lib"); + CmdArgs.push_back("-defaultlib:sycl-preview.lib"); else - CmdArgs.push_back("-defaultlib:sycl" SYCL_MAJOR_VERSION ".lib"); + CmdArgs.push_back("-defaultlib:sycl.lib"); } else { if (Args.hasArg(options::OPT_fpreview_breaking_changes)) - CmdArgs.push_back("-defaultlib:sycl" SYCL_MAJOR_VERSION - "-previewd.lib"); + CmdArgs.push_back("-defaultlib:sycl-previewd.lib"); else - CmdArgs.push_back("-defaultlib:sycl" SYCL_MAJOR_VERSION "d.lib"); + CmdArgs.push_back("-defaultlib:sycld.lib"); } CmdArgs.push_back("-defaultlib:sycl-devicelib-host.lib"); } diff --git a/sycl/source/CMakeLists.txt b/sycl/source/CMakeLists.txt index 0da1c5fa0cea2..ead8f2c83ab71 100644 --- a/sycl/source/CMakeLists.txt +++ b/sycl/source/CMakeLists.txt @@ -18,7 +18,7 @@ endif() function(add_sycl_rt_library LIB_NAME LIB_OBJ_NAME) # Add an optional argument so we can get the library name to # link with for Windows Debug version - cmake_parse_arguments(ARG "" "XPTI_LIB" "COMPILE_OPTIONS;SOURCES" ${ARGN}) + cmake_parse_arguments(ARG "" "XPTI_LIB;IMPLIB_NAME" "COMPILE_OPTIONS;SOURCES" ${ARGN}) add_library(${LIB_OBJ_NAME} OBJECT ${ARG_SOURCES}) add_library(${LIB_NAME} SHARED @@ -75,12 +75,12 @@ function(add_sycl_rt_library LIB_NAME LIB_OBJ_NAME) if (WIN32) include_directories(${LLVM_EXTERNAL_SYCL_SOURCE_DIR}/pi_win_proxy_loader) if(WIN_DUPE) - target_link_libraries(${LIB_NAME} PUBLIC pi_win_proxy_loaderd) + target_link_libraries(${LIB_NAME} PUBLIC pi_win_proxy_loaderd) set(MANIFEST_FILE_NAME "sycld.manifest") - else() - target_link_libraries(${LIB_NAME} PUBLIC pi_win_proxy_loader) + else() + target_link_libraries(${LIB_NAME} PUBLIC pi_win_proxy_loader) set(MANIFEST_FILE_NAME "sycl.manifest") - endif() + endif() # Embed manifest into the sycl.dll where pi_win_proxy_loader.dll is described as sycl.dll's private dll and will always be loaded from the same directory. # 0x2000: LOAD_LIBRARY_SAFE_CURRENT_DIRS flag. Using this flag means that loading dependency DLLs (of sycl.dll) # from the current directory is only allowed if it is under a directory in the Safe load list. @@ -92,6 +92,16 @@ function(add_sycl_rt_library LIB_NAME LIB_OBJ_NAME) if (WIN32) target_compile_definitions(${LIB_OBJ_NAME} PRIVATE __SYCL_BUILD_SYCL_DLL ) target_link_libraries(${LIB_NAME} PRIVATE shlwapi) + if (ARG_IMPLIB_NAME) + add_custom_command( + TARGET ${LIB_NAME} POST_BUILD + COMMAND ${CMAKE_COMMAND} -E copy + ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}/${LIB_NAME}.lib ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}/${ARG_IMPLIB_NAME}.lib + COMMENT "Creating version-agnostic copy of the import library.") + install( + FILES ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}/${ARG_IMPLIB_NAME}.lib + DESTINATION "lib${LLVM_LIBDIR_SUFFIX}" COMPONENT sycl) + endif() endif() if (MSVC) @@ -277,14 +287,14 @@ if (MSVC) set(WIN_DUPE "1") if (SYCL_ENABLE_XPTI_TRACING) - add_sycl_rt_library(sycl${SYCL_MAJOR_VERSION}d sycld_object XPTI_LIB xptid COMPILE_OPTIONS "/MDd" SOURCES ${SYCL_NON_PREVIEW_SOURCES}) + add_sycl_rt_library(sycl${SYCL_MAJOR_VERSION}d sycld_object XPTI_LIB xptid COMPILE_OPTIONS "/MDd" SOURCES ${SYCL_NON_PREVIEW_SOURCES} IMPLIB_NAME sycld) if(SYCL_ENABLE_MAJOR_RELEASE_PREVIEW_LIB) - add_sycl_rt_library(sycl${SYCL_MAJOR_VERSION}-previewd sycl-previewd_object XPTI_LIB xptid COMPILE_OPTIONS "/MDd" "/D__INTEL_PREVIEW_BREAKING_CHANGES" SOURCES ${SYCL_PREVIEW_SOURCES}) + add_sycl_rt_library(sycl${SYCL_MAJOR_VERSION}-previewd sycl-previewd_object XPTI_LIB xptid COMPILE_OPTIONS "/MDd" "/D__INTEL_PREVIEW_BREAKING_CHANGES" SOURCES ${SYCL_PREVIEW_SOURCES} IMPLIB_NAME sycl-previewd) endif() else() - add_sycl_rt_library(sycl${SYCL_MAJOR_VERSION}d sycld_object COMPILE_OPTIONS "/MDd" SOURCES ${SYCL_NON_PREVIEW_SOURCES}) + add_sycl_rt_library(sycl${SYCL_MAJOR_VERSION}d sycld_object COMPILE_OPTIONS "/MDd" SOURCES ${SYCL_NON_PREVIEW_SOURCES} IMPLIB_NAME sycld) if(SYCL_ENABLE_MAJOR_RELEASE_PREVIEW_LIB) - add_sycl_rt_library(sycl${SYCL_MAJOR_VERSION}-previewd sycl-previewd_object COMPILE_OPTIONS "/MDd" "/D__INTEL_PREVIEW_BREAKING_CHANGES" SOURCES ${SYCL_PREVIEW_SOURCES}) + add_sycl_rt_library(sycl${SYCL_MAJOR_VERSION}-previewd sycl-previewd_object COMPILE_OPTIONS "/MDd" "/D__INTEL_PREVIEW_BREAKING_CHANGES" SOURCES ${SYCL_PREVIEW_SOURCES} IMPLIB_NAME sycl-previewd) endif() endif() unset(WIN_DUPE) @@ -307,16 +317,18 @@ set(LIB_NAME "sycl${SYCL_MAJOR_VERSION}") else() set(LIB_NAME "sycl") endif() +# Version-agnostic name of the import library, has effect on Windows only. +set(IMPLIB_NAME "sycl") if (SYCL_ENABLE_XPTI_TRACING) - add_sycl_rt_library(${LIB_NAME} sycl_object XPTI_LIB xpti COMPILE_OPTIONS ${SYCL_EXTRA_OPTS} SOURCES ${SYCL_NON_PREVIEW_SOURCES}) + add_sycl_rt_library(${LIB_NAME} sycl_object XPTI_LIB xpti COMPILE_OPTIONS ${SYCL_EXTRA_OPTS} SOURCES ${SYCL_NON_PREVIEW_SOURCES} IMPLIB_NAME ${IMPLIB_NAME}) if(SYCL_ENABLE_MAJOR_RELEASE_PREVIEW_LIB) - add_sycl_rt_library(${LIB_NAME}-preview sycl-preview_object XPTI_LIB xpti COMPILE_OPTIONS ${SYCL_EXTRA_OPTS} "-D__INTEL_PREVIEW_BREAKING_CHANGES" SOURCES ${SYCL_PREVIEW_SOURCES}) + add_sycl_rt_library(${LIB_NAME}-preview sycl-preview_object XPTI_LIB xpti COMPILE_OPTIONS ${SYCL_EXTRA_OPTS} "-D__INTEL_PREVIEW_BREAKING_CHANGES" SOURCES ${SYCL_PREVIEW_SOURCES} IMPLIB_NAME ${IMPLIB_NAME}-preview) endif() else() - add_sycl_rt_library(${LIB_NAME} sycl_object COMPILE_OPTIONS ${SYCL_EXTRA_OPTS} SOURCES ${SYCL_NON_PREVIEW_SOURCES}) + add_sycl_rt_library(${LIB_NAME} sycl_object COMPILE_OPTIONS ${SYCL_EXTRA_OPTS} SOURCES ${SYCL_NON_PREVIEW_SOURCES} IMPLIB_NAME ${IMPLIB_NAME}) if(SYCL_ENABLE_MAJOR_RELEASE_PREVIEW_LIB) - add_sycl_rt_library(${LIB_NAME}-preview sycl-preview_object COMPILE_OPTIONS ${SYCL_EXTRA_OPTS} "-D__INTEL_PREVIEW_BREAKING_CHANGES" SOURCES ${SYCL_PREVIEW_SOURCES}) + add_sycl_rt_library(${LIB_NAME}-preview sycl-preview_object COMPILE_OPTIONS ${SYCL_EXTRA_OPTS} "-D__INTEL_PREVIEW_BREAKING_CHANGES" SOURCES ${SYCL_PREVIEW_SOURCES} IMPLIB_NAME ${IMPLIB_NAME}-preview) endif() endif() diff --git a/sycl/test-e2e/Basic/windows_version_agnostic_sycl_lib.cpp b/sycl/test-e2e/Basic/windows_version_agnostic_sycl_lib.cpp new file mode 100644 index 0000000000000..04d2d45c79950 --- /dev/null +++ b/sycl/test-e2e/Basic/windows_version_agnostic_sycl_lib.cpp @@ -0,0 +1,16 @@ +// REQUIRES: windows + +// RUN: %clangxx --driver-mode=cl /std:c++17 /EHsc -I%sycl_include -I%opencl_include_dir %s -o %t.out /link /defaultlib:%sycl_static_libs_dir/sycl.lib +// RUN: %{run} %t.out + +// This test checks that if program is linked with version-agnostic import library sycl.lib then sycl program works as expected. +// It is expected to crash if correct dll can't be loaded. + +#include +#include + +using namespace sycl; + +int main() { + queue q; +} From 8cdd92082134f365cffc96f21536708017763ab2 Mon Sep 17 00:00:00 2001 From: "Gainullin, Artur" Date: Tue, 5 Mar 2024 12:55:22 -0800 Subject: [PATCH 2/2] Revert clang driver changes --- clang/lib/Driver/CMakeLists.txt | 2 +- clang/lib/Driver/ToolChains/Clang.cpp | 15 +++++++++------ clang/lib/Driver/ToolChains/MSVC.cpp | 9 +++++---- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/clang/lib/Driver/CMakeLists.txt b/clang/lib/Driver/CMakeLists.txt index 2949b2a506887..d5d2bd4aab75e 100644 --- a/clang/lib/Driver/CMakeLists.txt +++ b/clang/lib/Driver/CMakeLists.txt @@ -18,7 +18,7 @@ if(WIN32) endif() # This must be in sync with llvm/sycl/CMakeLists.txt. -SET_SOURCE_FILES_PROPERTIES( ToolChains/Clang.cpp +SET_SOURCE_FILES_PROPERTIES( ToolChains/MSVC.cpp ToolChains/Clang.cpp PROPERTIES COMPILE_DEFINITIONS SYCL_MAJOR_VERSION="7" ) add_clang_library(clangDriver diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 638b9bbcb2390..9cafc66bd2ea2 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -5035,14 +5035,16 @@ static void ProcessVSRuntimeLibrary(const ArgList &Args, !Args.hasArg(options::OPT_nolibsycl)) { if (RTOptionID == options::OPT__SLASH_MDd) { if (Args.hasArg(options::OPT_fpreview_breaking_changes)) - CmdArgs.push_back("--dependent-lib=sycl-previewd"); + CmdArgs.push_back("--dependent-lib=sycl" SYCL_MAJOR_VERSION + "-previewd"); else - CmdArgs.push_back("--dependent-lib=sycld"); + CmdArgs.push_back("--dependent-lib=sycl" SYCL_MAJOR_VERSION "d"); } else { if (Args.hasArg(options::OPT_fpreview_breaking_changes)) - CmdArgs.push_back("--dependent-lib=sycl-preview"); + CmdArgs.push_back("--dependent-lib=sycl" SYCL_MAJOR_VERSION + "-preview"); else - CmdArgs.push_back("--dependent-lib=sycl"); + CmdArgs.push_back("--dependent-lib=sycl" SYCL_MAJOR_VERSION); } CmdArgs.push_back("--dependent-lib=sycl-devicelib-host"); } @@ -6596,9 +6598,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, if (!D.IsCLMode() && TC.getTriple().isWindowsMSVCEnvironment()) { if (isDependentLibAdded(Args, "msvcrtd")) { if (Args.hasArg(options::OPT_fpreview_breaking_changes)) - CmdArgs.push_back("--dependent-lib=sycl-previewd"); + CmdArgs.push_back("--dependent-lib=sycl" SYCL_MAJOR_VERSION + "-previewd"); else - CmdArgs.push_back("--dependent-lib=sycld"); + CmdArgs.push_back("--dependent-lib=sycl" SYCL_MAJOR_VERSION "d"); } } else if (!D.IsCLMode() && TC.getTriple().isWindowsGNUEnvironment()) { if (Args.hasArg(options::OPT_fpreview_breaking_changes)) diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp index ea695abda3bfa..5e08965320004 100644 --- a/clang/lib/Driver/ToolChains/MSVC.cpp +++ b/clang/lib/Driver/ToolChains/MSVC.cpp @@ -138,14 +138,15 @@ void visualstudio::Linker::ConstructJob(Compilation &C, const JobAction &JA, if (!Args.hasArg(options::OPT__SLASH_MDd) && !isDependentLibAdded(Args, "msvcrtd")) { if (Args.hasArg(options::OPT_fpreview_breaking_changes)) - CmdArgs.push_back("-defaultlib:sycl-preview.lib"); + CmdArgs.push_back("-defaultlib:sycl" SYCL_MAJOR_VERSION "-preview.lib"); else - CmdArgs.push_back("-defaultlib:sycl.lib"); + CmdArgs.push_back("-defaultlib:sycl" SYCL_MAJOR_VERSION ".lib"); } else { if (Args.hasArg(options::OPT_fpreview_breaking_changes)) - CmdArgs.push_back("-defaultlib:sycl-previewd.lib"); + CmdArgs.push_back("-defaultlib:sycl" SYCL_MAJOR_VERSION + "-previewd.lib"); else - CmdArgs.push_back("-defaultlib:sycld.lib"); + CmdArgs.push_back("-defaultlib:sycl" SYCL_MAJOR_VERSION "d.lib"); } CmdArgs.push_back("-defaultlib:sycl-devicelib-host.lib"); }