From 4960f0924fa042d003fe886bfb00dd9c39fab2d2 Mon Sep 17 00:00:00 2001 From: Vladimir Lazarev Date: Thu, 12 Aug 2021 11:29:26 +0300 Subject: [PATCH 1/2] [NFC][SYCL] Fix PIMock initialization in unit tests The problem comes from global initialization which is dependent on another global which happens to be not initialized yet in this case (the order is non-deterministic by standard). --- sycl/unittests/pi/EnqueueMemTest.cpp | 5 ++--- sycl/unittests/pi/PlatformTest.cpp | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/sycl/unittests/pi/EnqueueMemTest.cpp b/sycl/unittests/pi/EnqueueMemTest.cpp index 8abf016e5b322..fe007bcce8945 100644 --- a/sycl/unittests/pi/EnqueueMemTest.cpp +++ b/sycl/unittests/pi/EnqueueMemTest.cpp @@ -105,10 +105,9 @@ class EnqueueMemTest : public testing::TestWithParam { } }; -static std::vector Plugins = pi::initializeAndRemoveInvalid(); - INSTANTIATE_TEST_CASE_P( - EnqueueMemTestImpl, EnqueueMemTest, testing::ValuesIn(Plugins), + EnqueueMemTestImpl, EnqueueMemTest, + testing::ValuesIn(pi::initializeAndRemoveInvalid()), [](const testing::TestParamInfo &info) { return pi::GetBackendString(info.param.getBackend()); }); diff --git a/sycl/unittests/pi/PlatformTest.cpp b/sycl/unittests/pi/PlatformTest.cpp index c3d04721cc992..edb50852a749e 100644 --- a/sycl/unittests/pi/PlatformTest.cpp +++ b/sycl/unittests/pi/PlatformTest.cpp @@ -62,10 +62,9 @@ class PlatformTest : public testing::TestWithParam { } }; -static std::vector Plugins = pi::initializeAndRemoveInvalid(); - INSTANTIATE_TEST_CASE_P( - PlatformTestImpl, PlatformTest, testing::ValuesIn(Plugins), + PlatformTestImpl, PlatformTest, + testing::ValuesIn(pi::initializeAndRemoveInvalid()), [](const testing::TestParamInfo &info) { return pi::GetBackendString(info.param.getBackend()); }); From 6f4b4f3ed52fedb1b7d299a818e0a04ac78e9f8d Mon Sep 17 00:00:00 2001 From: Vladimir Lazarev Date: Mon, 16 Aug 2021 12:03:26 +0300 Subject: [PATCH 2/2] Move global maps to static locals in free functions That allows to avoid initialization races on usages. --- sycl/source/detail/allowlist.cpp | 8 +++--- sycl/source/detail/config.cpp | 28 +++++++++++++++++++-- sycl/source/detail/config.hpp | 16 +++--------- sycl/source/detail/device_filter.cpp | 12 ++++----- sycl/unittests/allowlist/ParseAllowList.cpp | 6 ++--- 5 files changed, 42 insertions(+), 28 deletions(-) diff --git a/sycl/source/detail/allowlist.cpp b/sycl/source/detail/allowlist.cpp index 22019dfdae482..fe8edecc5f2a4 100644 --- a/sycl/source/detail/allowlist.cpp +++ b/sycl/source/detail/allowlist.cpp @@ -148,8 +148,8 @@ AllowListParsedT parseAllowList(const std::string &AllowListRaw) { // check that values of keys, which should have some fixed format, are // valid. E.g., for BackendName key, the allowed values are only ones // described in SyclBeMap - ValidateEnumValues(BackendNameKeyName, SyclBeMap); - ValidateEnumValues(DeviceTypeKeyName, SyclDeviceTypeMap); + ValidateEnumValues(BackendNameKeyName, getSyclBeMap()); + ValidateEnumValues(DeviceTypeKeyName, getSyclDeviceTypeMap()); if (Key == DeviceVendorIdKeyName) { // DeviceVendorId should have hex format @@ -310,7 +310,7 @@ void applyAllowList(std::vector &PiDevices, // get BackendName value and put it to DeviceDesc sycl::backend Backend = Plugin.getBackend(); - for (const auto &SyclBe : SyclBeMap) { + for (const auto &SyclBe : getSyclBeMap()) { if (SyclBe.second == Backend) { DeviceDesc.emplace(BackendNameKeyName, SyclBe.first); break; @@ -336,7 +336,7 @@ void applyAllowList(std::vector &PiDevices, sizeof(RT::PiDeviceType), &PiDevType, nullptr); sycl::info::device_type DeviceType = pi::cast(PiDevType); - for (const auto &SyclDeviceType : SyclDeviceTypeMap) { + for (const auto &SyclDeviceType : getSyclDeviceTypeMap()) { if (SyclDeviceType.second == DeviceType) { const auto &DeviceTypeValue = SyclDeviceType.first; DeviceDesc[DeviceTypeKeyName] = DeviceTypeValue; diff --git a/sycl/source/detail/config.cpp b/sycl/source/detail/config.cpp index 9a9f4963803c1..7f280e1b61adc 100644 --- a/sycl/source/detail/config.cpp +++ b/sycl/source/detail/config.cpp @@ -111,6 +111,30 @@ void dumpConfig() { #undef CONFIG } -} // __SYCL_INLINE_NAMESPACE(cl) -} // namespace sycl +// Array is used by SYCL_DEVICE_FILTER and SYCL_DEVICE_ALLOWLIST +const std::array, 5> & +getSyclDeviceTypeMap() { + static const std::array, 5> + SyclDeviceTypeMap = {{{"host", info::device_type::host}, + {"cpu", info::device_type::cpu}, + {"gpu", info::device_type::gpu}, + {"acc", info::device_type::accelerator}, + {"*", info::device_type::all}}}; + return SyclDeviceTypeMap; +} + +// Array is used by SYCL_DEVICE_FILTER and SYCL_DEVICE_ALLOWLIST +const std::array, 6> &getSyclBeMap() { + static const std::array, 6> SyclBeMap = { + {{"host", backend::host}, + {"opencl", backend::opencl}, + {"level_zero", backend::level_zero}, + {"cuda", backend::cuda}, + {"rocm", backend::rocm}, + {"*", backend::all}}}; + return SyclBeMap; +} + } // namespace detail +} // namespace sycl +} // __SYCL_INLINE_NAMESPACE(cl) diff --git a/sycl/source/detail/config.hpp b/sycl/source/detail/config.hpp index b19152c912b1f..63ad7a43299b0 100644 --- a/sycl/source/detail/config.hpp +++ b/sycl/source/detail/config.hpp @@ -177,21 +177,11 @@ template <> class SYCLConfig { }; // Array is used by SYCL_DEVICE_FILTER and SYCL_DEVICE_ALLOWLIST -static const std::array, 5> - SyclDeviceTypeMap = {{{"host", info::device_type::host}, - {"cpu", info::device_type::cpu}, - {"gpu", info::device_type::gpu}, - {"acc", info::device_type::accelerator}, - {"*", info::device_type::all}}}; +const std::array, 5> & +getSyclDeviceTypeMap(); // Array is used by SYCL_DEVICE_FILTER and SYCL_DEVICE_ALLOWLIST -static const std::array, 6> SyclBeMap = { - {{"host", backend::host}, - {"opencl", backend::opencl}, - {"level_zero", backend::level_zero}, - {"cuda", backend::cuda}, - {"rocm", backend::rocm}, - {"*", backend::all}}}; +const std::array, 6> &getSyclBeMap(); template <> class SYCLConfig { using BaseT = SYCLConfigBase; diff --git a/sycl/source/detail/device_filter.cpp b/sycl/source/detail/device_filter.cpp index 3b0847d105571..6ff840c578290 100644 --- a/sycl/source/detail/device_filter.cpp +++ b/sycl/source/detail/device_filter.cpp @@ -30,11 +30,11 @@ device_filter::device_filter(const std::string &FilterString) { // Handle the optional 1st field of the filter, backend // Check if the first entry matches with a known backend type - auto It = - std::find_if(std::begin(SyclBeMap), std::end(SyclBeMap), findElement); + auto It = std::find_if(std::begin(getSyclBeMap()), std::end(getSyclBeMap()), + findElement); // If no match is found, set the backend type backend::all // which actually means 'any backend' will be a match. - if (It == SyclBeMap.end()) + if (It == getSyclBeMap().end()) Backend = backend::all; else { Backend = It->second; @@ -49,11 +49,11 @@ device_filter::device_filter(const std::string &FilterString) { if (Cursor >= FilterString.size()) { DeviceType = info::device_type::all; } else { - auto Iter = std::find_if(std::begin(SyclDeviceTypeMap), - std::end(SyclDeviceTypeMap), findElement); + auto Iter = std::find_if(std::begin(getSyclDeviceTypeMap()), + std::end(getSyclDeviceTypeMap()), findElement); // If no match is found, set device_type 'all', // which actually means 'any device_type' will be a match. - if (Iter == SyclDeviceTypeMap.end()) + if (Iter == getSyclDeviceTypeMap().end()) DeviceType = info::device_type::all; else { DeviceType = Iter->second; diff --git a/sycl/unittests/allowlist/ParseAllowList.cpp b/sycl/unittests/allowlist/ParseAllowList.cpp index 0791e48c586ac..4c417618c4a62 100644 --- a/sycl/unittests/allowlist/ParseAllowList.cpp +++ b/sycl/unittests/allowlist/ParseAllowList.cpp @@ -7,7 +7,7 @@ //===----------------------------------------------------------------------===// #include -#include // for SyclBeMap and SyclDeviceTypeMap +#include // for getSyclBeMap() and getSyclDeviceTypeMap() #include @@ -157,7 +157,7 @@ TEST(ParseAllowListTests, CheckMissingClosedDoubleCurlyBracesAreHandled) { TEST(ParseAllowListTests, CheckAllValidBackendNameValuesAreProcessed) { std::string AllowList; - for (const auto &SyclBe : sycl::detail::SyclBeMap) { + for (const auto &SyclBe : sycl::detail::getSyclBeMap()) { if (!AllowList.empty()) AllowList += "|"; AllowList += "BackendName:" + SyclBe.first; @@ -173,7 +173,7 @@ TEST(ParseAllowListTests, CheckAllValidBackendNameValuesAreProcessed) { TEST(ParseAllowListTests, CheckAllValidDeviceTypeValuesAreProcessed) { std::string AllowList; - for (const auto &SyclDeviceType : sycl::detail::SyclDeviceTypeMap) { + for (const auto &SyclDeviceType : sycl::detail::getSyclDeviceTypeMap()) { if (!AllowList.empty()) AllowList += "|"; AllowList += "DeviceType:" + SyclDeviceType.first;