From 6d87519e1f1fa7a431bc38ea534f7151b7d920c9 Mon Sep 17 00:00:00 2001 From: Vyacheslav N Klochkov Date: Tue, 15 Sep 2020 21:28:26 -0700 Subject: [PATCH 01/11] [SYCL] Add a static_assert/check that app and sycl use consistent C++ RT on win SYCL library is designed such a way that STL objects must cross the sycl.dll boundary, which is guaranteed to work safe on Windows only if the runtime in the app using sycl.dll and in sycl.dll is the same and is dynamic. It is not possible to implement safe approach for using sycl libraries built/linked with static C++ RT as it would cause having multiple copies of C++ objects (such as scheduler, etc), which are supposed to be singletones. This check reports a compile-time error when user tries to compile the application using sycl.dll with /MT or /MTd switches implying using static C++ runtime libcmt[d].lib. sycl.dll is built with /MD and sycld.dll is built with /MDd. Thus /MD and /MDd are allowed, and /MT /MTd switches are prohibited now. --- sycl/include/CL/sycl/stl.hpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/sycl/include/CL/sycl/stl.hpp b/sycl/include/CL/sycl/stl.hpp index f57fc0674c8fe..b580ac5c1b1ac 100644 --- a/sycl/include/CL/sycl/stl.hpp +++ b/sycl/include/CL/sycl/stl.hpp @@ -22,6 +22,27 @@ __SYCL_INLINE_NAMESPACE(cl) { namespace sycl { +#ifdef _WIN32 +namespace detail { +// SYCL library is designed such a way that STL objects cross DLL boundary, +// which is not guaranteed to work and considered not safe. +// Using same dynamic C++ runtime library for sycl[d].dll and for +// the application using sycl[d].dll is guaranteed to work properly. +inline constexpr bool isMSVCDynamicCXXRuntime() { +// The options /MD and /MDd that make the code to use dynamic runtime also +// define the _DLL macro. +#ifdef _DLL + return true; +#else + return false; +#endif +} +static_assert(isMSVCDynamicCXXRuntime(), + "SYCL library is designed to work with dynamic C++ runtime, " + "please use /MD or /MDd switches."); +} // namespace detail +#endif // _WIN32 + template < class T, class Alloc = std::allocator > using vector_class = std::vector; From b27272872a7dde4aa9119ed56b4757ab8b9f5380 Mon Sep 17 00:00:00 2001 From: Vyacheslav N Klochkov Date: Tue, 15 Sep 2020 22:18:00 -0700 Subject: [PATCH 02/11] clang-format That is odd that clang-format required me to do fixes that I did not really touch (only had changed preceding the problem places). --- sycl/include/CL/sycl/stl.hpp | 39 +++++++++++++++++------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/sycl/include/CL/sycl/stl.hpp b/sycl/include/CL/sycl/stl.hpp index b580ac5c1b1ac..b71ffc86e6dbb 100644 --- a/sycl/include/CL/sycl/stl.hpp +++ b/sycl/include/CL/sycl/stl.hpp @@ -25,8 +25,8 @@ namespace sycl { #ifdef _WIN32 namespace detail { // SYCL library is designed such a way that STL objects cross DLL boundary, -// which is not guaranteed to work and considered not safe. -// Using same dynamic C++ runtime library for sycl[d].dll and for +// which is not guaranteed to work and considered not safe in general. +// Only using same dynamic C++ runtime library for sycl[d].dll and for // the application using sycl[d].dll is guaranteed to work properly. inline constexpr bool isMSVCDynamicCXXRuntime() { // The options /MD and /MDd that make the code to use dynamic runtime also @@ -43,34 +43,31 @@ static_assert(isMSVCDynamicCXXRuntime(), } // namespace detail #endif // _WIN32 - template < class T, class Alloc = std::allocator > - using vector_class = std::vector; +template > +using vector_class = std::vector; - using string_class = std::string; +using string_class = std::string; - template - using function_class = std::function; +template using function_class = std::function; - using mutex_class = std::mutex; +using mutex_class = std::mutex; - template > - using unique_ptr_class = std::unique_ptr; +template > +using unique_ptr_class = std::unique_ptr; - template - using shared_ptr_class = std::shared_ptr; +template using shared_ptr_class = std::shared_ptr; - template - using weak_ptr_class = std::weak_ptr; +template using weak_ptr_class = std::weak_ptr; - template - using hash_class = std::hash; +template using hash_class = std::hash; - using exception_ptr_class = std::exception_ptr; +using exception_ptr_class = std::exception_ptr; + +template +unique_ptr_class make_unique_ptr(ArgsT &&... Args) { + return unique_ptr_class(new T(std::forward(Args)...)); +} - template - unique_ptr_class make_unique_ptr(ArgsT &&... Args) { - return unique_ptr_class(new T(std::forward(Args)...)); - } } // sycl } // cl From 6c5ef66551a8fbb671f1ad391d5d4fd21f1736c9 Mon Sep 17 00:00:00 2001 From: Vyacheslav N Klochkov Date: Wed, 16 Sep 2020 21:07:41 -0700 Subject: [PATCH 03/11] [SYCL][Driver] Use dynamic C++ runtime by default in clang on Windows --- clang/lib/Driver/ToolChains/Clang.cpp | 12 ++++++++++-- clang/lib/Driver/ToolChains/MSVC.cpp | 8 ++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 82aad7db78070..a3d2cd65633b4 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -6272,6 +6272,15 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, if (getToolChain().getTriple().getSubArch() == llvm::Triple::SPIRSubArch_fpga) CmdArgs.push_back("-D__ENABLE_USM_ADDR_SPACE__"); + +#if defined(_WIN32) + // SYCL library is guaranteed to work correctly only with dynamic runtime. + if (!D.IsCLMode()) { + CmdArgs.push_back("-D_MT"); + CmdArgs.push_back("-D_DLL"); + CmdArgs.push_back("--dependent-lib=msvcrt"); + } +#endif // _WIN32 } if (IsHIP) @@ -6835,8 +6844,7 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID InputType, // Add SYCL dependent library if (Args.hasArg(options::OPT_fsycl) && !Args.hasArg(options::OPT_nolibsycl)) { - if (RTOptionID == options::OPT__SLASH_MDd || - RTOptionID == options::OPT__SLASH_MTd) + if (RTOptionID == options::OPT__SLASH_MDd) CmdArgs.push_back("--dependent-lib=sycld"); else CmdArgs.push_back("--dependent-lib=sycl"); diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp index 01f68e38d65b6..447b4a9e87912 100644 --- a/clang/lib/Driver/ToolChains/MSVC.cpp +++ b/clang/lib/Driver/ToolChains/MSVC.cpp @@ -370,8 +370,12 @@ void visualstudio::Linker::ConstructJob(Compilation &C, const JobAction &JA, Args.MakeArgString(std::string("-out:") + Output.getFilename())); if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles) && - !C.getDriver().IsCLMode()) - CmdArgs.push_back("-defaultlib:libcmt"); + !C.getDriver().IsCLMode()) { + if (Args.hasArg(options::OPT_fsycl) && !Args.hasArg(options::OPT_nolibsycl)) + CmdArgs.push_back("-defaultlib:msvcrt"); + else + CmdArgs.push_back("-defaultlib:libcmt"); + } if (!C.getDriver().IsCLMode() && !Args.hasArg(options::OPT_nostdlib) && Args.hasArg(options::OPT_fsycl) && !Args.hasArg(options::OPT_nolibsycl)) { From b060709dc1dfdcb7fbf664dacfafa68624da99b3 Mon Sep 17 00:00:00 2001 From: Vyacheslav N Klochkov Date: Thu, 17 Sep 2020 13:51:41 -0700 Subject: [PATCH 04/11] Additional fix for clang++ driver (respond to reviewer's comment) --- clang/lib/Driver/ToolChains/Clang.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index a3d2cd65633b4..ef2028bf31fd6 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -6273,14 +6273,12 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, llvm::Triple::SPIRSubArch_fpga) CmdArgs.push_back("-D__ENABLE_USM_ADDR_SPACE__"); -#if defined(_WIN32) // SYCL library is guaranteed to work correctly only with dynamic runtime. - if (!D.IsCLMode()) { + if (!D.IsCLMode() && C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment()) { CmdArgs.push_back("-D_MT"); CmdArgs.push_back("-D_DLL"); CmdArgs.push_back("--dependent-lib=msvcrt"); } -#endif // _WIN32 } if (IsHIP) From cc3fbecea825411d5ab6413808ca55084644ca1f Mon Sep 17 00:00:00 2001 From: Vyacheslav N Klochkov Date: Thu, 17 Sep 2020 14:00:17 -0700 Subject: [PATCH 05/11] clang-format --- clang/lib/Driver/ToolChains/Clang.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index d53bb46ad0fe6..17b779f75f743 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -6275,7 +6275,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, CmdArgs.push_back("-D__ENABLE_USM_ADDR_SPACE__"); // SYCL library is guaranteed to work correctly only with dynamic runtime. - if (!D.IsCLMode() && C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment()) { + if (!D.IsCLMode() && + C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment()) { CmdArgs.push_back("-D_MT"); CmdArgs.push_back("-D_DLL"); CmdArgs.push_back("--dependent-lib=msvcrt"); From 8c327109ba16dbd99686dd3b3de4f583c43655dc Mon Sep 17 00:00:00 2001 From: Vyacheslav N Klochkov Date: Thu, 17 Sep 2020 14:05:56 -0700 Subject: [PATCH 06/11] clang-format --- clang/lib/Driver/ToolChains/Clang.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 17b779f75f743..c2c04324fd43e 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -6276,7 +6276,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, // SYCL library is guaranteed to work correctly only with dynamic runtime. if (!D.IsCLMode() && - C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment()) { + C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment()) { CmdArgs.push_back("-D_MT"); CmdArgs.push_back("-D_DLL"); CmdArgs.push_back("--dependent-lib=msvcrt"); From 1bbed538244b215f026e655ec4202adbd2d5676d Mon Sep 17 00:00:00 2001 From: Vyacheslav N Klochkov Date: Fri, 18 Sep 2020 15:15:31 -0700 Subject: [PATCH 07/11] Temporarily cancelled the changes in sycl/stl.hpp - will be moved to separate PR --- sycl/include/CL/sycl/stl.hpp | 56 ++++++++++++------------------------ 1 file changed, 19 insertions(+), 37 deletions(-) diff --git a/sycl/include/CL/sycl/stl.hpp b/sycl/include/CL/sycl/stl.hpp index b71ffc86e6dbb..f57fc0674c8fe 100644 --- a/sycl/include/CL/sycl/stl.hpp +++ b/sycl/include/CL/sycl/stl.hpp @@ -22,52 +22,34 @@ __SYCL_INLINE_NAMESPACE(cl) { namespace sycl { -#ifdef _WIN32 -namespace detail { -// SYCL library is designed such a way that STL objects cross DLL boundary, -// which is not guaranteed to work and considered not safe in general. -// Only using same dynamic C++ runtime library for sycl[d].dll and for -// the application using sycl[d].dll is guaranteed to work properly. -inline constexpr bool isMSVCDynamicCXXRuntime() { -// The options /MD and /MDd that make the code to use dynamic runtime also -// define the _DLL macro. -#ifdef _DLL - return true; -#else - return false; -#endif -} -static_assert(isMSVCDynamicCXXRuntime(), - "SYCL library is designed to work with dynamic C++ runtime, " - "please use /MD or /MDd switches."); -} // namespace detail -#endif // _WIN32 + template < class T, class Alloc = std::allocator > + using vector_class = std::vector; -template > -using vector_class = std::vector; + using string_class = std::string; -using string_class = std::string; + template + using function_class = std::function; -template using function_class = std::function; + using mutex_class = std::mutex; -using mutex_class = std::mutex; + template > + using unique_ptr_class = std::unique_ptr; -template > -using unique_ptr_class = std::unique_ptr; + template + using shared_ptr_class = std::shared_ptr; -template using shared_ptr_class = std::shared_ptr; + template + using weak_ptr_class = std::weak_ptr; -template using weak_ptr_class = std::weak_ptr; + template + using hash_class = std::hash; -template using hash_class = std::hash; - -using exception_ptr_class = std::exception_ptr; - -template -unique_ptr_class make_unique_ptr(ArgsT &&... Args) { - return unique_ptr_class(new T(std::forward(Args)...)); -} + using exception_ptr_class = std::exception_ptr; + template + unique_ptr_class make_unique_ptr(ArgsT &&... Args) { + return unique_ptr_class(new T(std::forward(Args)...)); + } } // sycl } // cl From f55b02d8237cd111c3f3bb63b43ef635347d99b1 Mon Sep 17 00:00:00 2001 From: Vyacheslav N Klochkov Date: Fri, 18 Sep 2020 15:17:02 -0700 Subject: [PATCH 08/11] Additional fix for clang - use /MD for to -fsycl-device-only as well --- clang/lib/Driver/ToolChains/Clang.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index c2c04324fd43e..7059c1961a536 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -6273,13 +6273,19 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, if (getToolChain().getTriple().getSubArch() == llvm::Triple::SPIRSubArch_fpga) CmdArgs.push_back("-D__ENABLE_USM_ADDR_SPACE__"); + } - // SYCL library is guaranteed to work correctly only with dynamic runtime. - if (!D.IsCLMode() && - C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment()) { + if ((IsSYCL || UseSYCLTriple) && !D.IsCLMode()) { + // SYCL library is guaranteed to work correctly only with dynamic + // MSVC runtime. + llvm::Triple AuxT = C.getDefaultToolChain().getTriple(); + if (Args.hasFlag(options::OPT_fsycl_device_only, OptSpecifier(), false)) + AuxT = llvm::Triple(llvm::sys::getProcessTriple()); + if (AuxT.isWindowsMSVCEnvironment()) { CmdArgs.push_back("-D_MT"); CmdArgs.push_back("-D_DLL"); - CmdArgs.push_back("--dependent-lib=msvcrt"); + if (IsSYCL && !IsSYCLOffloadDevice && SYCLDeviceInput) + CmdArgs.push_back("--dependent-lib=msvcrt"); } } From 244290b10a2a8f709d2b0e06c47cf6bd7ef4bf9a Mon Sep 17 00:00:00 2001 From: Vyacheslav N Klochkov Date: Fri, 18 Sep 2020 15:45:15 -0700 Subject: [PATCH 09/11] Added a LIT test for the changes in driver --- clang/test/Driver/sycl-MD-default.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/clang/test/Driver/sycl-MD-default.cpp b/clang/test/Driver/sycl-MD-default.cpp index b7ffd42f6a274..b368844f72781 100644 --- a/clang/test/Driver/sycl-MD-default.cpp +++ b/clang/test/Driver/sycl-MD-default.cpp @@ -1,5 +1,9 @@ // REQUIRES: clang-driver +// RUN: %clang -### -fsycl -c %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHK-DEFAULT %s +// RUN: %clangxx -### -fsycl -c %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHK-DEFAULT %s // RUN: %clang_cl -### -fsycl -c %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHK-DEFAULT %s // RUN: %clang_cl -### -MD -fsycl -c %s 2>&1 \ @@ -9,6 +13,12 @@ // CHK-DEFAULT: "-D_MT" "-D_DLL" // CHK-DEFAULT: "--dependent-lib=msvcrt{{d*}}" +// RUN: %clang -### -fsycl-device-only -c %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHK-DEFAULT-DEF %s +// RUN: %clangxx -### -fsycl -c %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHK-DEFAULT-DEF %s +// CHK-DEFAULT-DEF: "-D_MT" "-D_DLL" + // RUN: %clang_cl -### -MT -fsycl -c %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHK-ERROR %s // RUN: %clang_cl -### -MTd -fsycl -c %s 2>&1 \ From 098b878398943e77db20d78b4341aa0b252fa265 Mon Sep 17 00:00:00 2001 From: Vyacheslav N Klochkov Date: Fri, 18 Sep 2020 19:17:10 -0700 Subject: [PATCH 10/11] Do not pass -D_DLL and -D_MT to device compilation. Otherwise, the errors reported during device code compilation because device code cannot handle implib call. --- clang/lib/Driver/ToolChains/Clang.cpp | 27 +++++++++++++-------------- clang/test/Driver/sycl-MD-default.cpp | 6 ------ 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 7059c1961a536..532909e8a0354 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -6258,6 +6258,19 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, if (Args.hasFlag(options::OPT_fsycl_esimd, options::OPT_fno_sycl_esimd, false)) CmdArgs.push_back("-fsycl-explicit-simd"); + + if (!D.IsCLMode()) { + // SYCL library is guaranteed to work correctly only with dynamic + // MSVC runtime. + llvm::Triple AuxT = C.getDefaultToolChain().getTriple(); + if (Args.hasFlag(options::OPT_fsycl_device_only, OptSpecifier(), false)) + AuxT = llvm::Triple(llvm::sys::getProcessTriple()); + if (AuxT.isWindowsMSVCEnvironment()) { + CmdArgs.push_back("-D_MT"); + CmdArgs.push_back("-D_DLL"); + CmdArgs.push_back("--dependent-lib=msvcrt"); + } + } } if (IsSYCLOffloadDevice && JA.getType() == types::TY_SYCL_Header) { // Generating a SYCL Header @@ -6275,20 +6288,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, CmdArgs.push_back("-D__ENABLE_USM_ADDR_SPACE__"); } - if ((IsSYCL || UseSYCLTriple) && !D.IsCLMode()) { - // SYCL library is guaranteed to work correctly only with dynamic - // MSVC runtime. - llvm::Triple AuxT = C.getDefaultToolChain().getTriple(); - if (Args.hasFlag(options::OPT_fsycl_device_only, OptSpecifier(), false)) - AuxT = llvm::Triple(llvm::sys::getProcessTriple()); - if (AuxT.isWindowsMSVCEnvironment()) { - CmdArgs.push_back("-D_MT"); - CmdArgs.push_back("-D_DLL"); - if (IsSYCL && !IsSYCLOffloadDevice && SYCLDeviceInput) - CmdArgs.push_back("--dependent-lib=msvcrt"); - } - } - if (IsHIP) CmdArgs.push_back("-fcuda-allow-variadic-functions"); diff --git a/clang/test/Driver/sycl-MD-default.cpp b/clang/test/Driver/sycl-MD-default.cpp index b368844f72781..a24575f5eeb02 100644 --- a/clang/test/Driver/sycl-MD-default.cpp +++ b/clang/test/Driver/sycl-MD-default.cpp @@ -13,12 +13,6 @@ // CHK-DEFAULT: "-D_MT" "-D_DLL" // CHK-DEFAULT: "--dependent-lib=msvcrt{{d*}}" -// RUN: %clang -### -fsycl-device-only -c %s 2>&1 \ -// RUN: | FileCheck -check-prefix=CHK-DEFAULT-DEF %s -// RUN: %clangxx -### -fsycl -c %s 2>&1 \ -// RUN: | FileCheck -check-prefix=CHK-DEFAULT-DEF %s -// CHK-DEFAULT-DEF: "-D_MT" "-D_DLL" - // RUN: %clang_cl -### -MT -fsycl -c %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHK-ERROR %s // RUN: %clang_cl -### -MTd -fsycl -c %s 2>&1 \ From 65a03440f5742e475667f22f590d90bb56b57ce8 Mon Sep 17 00:00:00 2001 From: Vyacheslav N Klochkov Date: Fri, 18 Sep 2020 22:13:59 -0700 Subject: [PATCH 11/11] [SYCL][LIT] Set the target for windows specific clang driver test --- clang/test/Driver/sycl-MD-default.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/Driver/sycl-MD-default.cpp b/clang/test/Driver/sycl-MD-default.cpp index a24575f5eeb02..5e88a48c76f54 100644 --- a/clang/test/Driver/sycl-MD-default.cpp +++ b/clang/test/Driver/sycl-MD-default.cpp @@ -1,8 +1,8 @@ // REQUIRES: clang-driver -// RUN: %clang -### -fsycl -c %s 2>&1 \ +// RUN: %clang -### -fsycl -c -target x86_64-unknown-windows-msvc %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHK-DEFAULT %s -// RUN: %clangxx -### -fsycl -c %s 2>&1 \ +// RUN: %clangxx -### -fsycl -c -target x86_64-unknown-windows-msvc %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHK-DEFAULT %s // RUN: %clang_cl -### -fsycl -c %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHK-DEFAULT %s