Skip to content

Commit f300e05

Browse files
[SYCL] Fix enabling of persistent device code cache in unit-tests (#6062)
The persistent device code cache currently only parse the configurations indicating if it is enabled once, caching the result. This can cause unit-tests for the cache to fail as they may be run after other tests that may have populated the cached result. To fix this, the persistent code cache now exposes a new function forcing the configuration to be reparsed. Additionally, the unit-tests will reset the SYCL_CACHE_PERSISTENT after each test case if it was set during. Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
1 parent 9de5a74 commit f300e05

File tree

3 files changed

+74
-15
lines changed

3 files changed

+74
-15
lines changed

sycl/source/detail/persistent_device_code_cache.cpp

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9-
#include <cstdio>
109
#include <detail/device_impl.hpp>
1110
#include <detail/persistent_device_code_cache.hpp>
1211
#include <detail/plugin.hpp>
1312
#include <detail/program_manager/program_manager.hpp>
1413

14+
#include <cstdio>
15+
#include <optional>
16+
1517
#if defined(__SYCL_RT_OS_LINUX)
1618
#include <unistd.h>
1719
#else
@@ -379,11 +381,28 @@ static bool parsePersistentCacheConfig() {
379381
return Ret;
380382
}
381383

384+
/* Cached static variable signalling if the persistent cache is enabled.
385+
* The variable can have three values:
386+
* - None : The configuration has not been parsed.
387+
* - true : The persistent cache is enabled.
388+
* - false : The persistent cache is disabled.
389+
*/
390+
static std::optional<bool> CacheIsEnabled;
391+
392+
/* Forces a reparsing of the information used to determine if the persistent
393+
* cache is enabled. This is primarily used for unit-testing where the
394+
* corresponding configuration variable is set by the individual tests.
395+
*/
396+
void PersistentDeviceCodeCache::reparseConfig() {
397+
CacheIsEnabled = parsePersistentCacheConfig();
398+
}
399+
382400
/* Returns true if persistent cache is enabled.
383401
*/
384402
bool PersistentDeviceCodeCache::isEnabled() {
385-
static bool Val = parsePersistentCacheConfig();
386-
return Val;
403+
if (!CacheIsEnabled)
404+
reparseConfig();
405+
return *CacheIsEnabled;
387406
}
388407

389408
/* Returns path for device code cache root directory

sycl/source/detail/persistent_device_code_cache.hpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,12 @@ class PersistentDeviceCodeCache {
184184
const std::string &BuildOptionsString,
185185
const RT::PiProgram &NativePrg);
186186

187+
/* Forces a reparsing of the information used to determine if the persistent
188+
* cache is enabled. This is primarily used for unit-testing where the
189+
* corresponding configuration variable is set by the individual tests.
190+
*/
191+
static void reparseConfig();
192+
187193
/* Sends message to std:cerr stream when SYCL_CACHE_TRACE environemnt is set*/
188194
static void trace(const std::string &msg) {
189195
static const char *TraceEnabled = SYCLConfig<SYCL_CACHE_TRACE>::get();

sycl/unittests/kernel-and-program/PersistentDeviceCodeCache.cpp

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <gtest/gtest.h>
1717
#include <helpers/PiMock.hpp>
1818
#include <llvm/Support/FileSystem.h>
19+
#include <optional>
1920
#include <vector>
2021

2122
#define ASSERT_NO_ERROR(x) \
@@ -28,9 +29,12 @@
2829
// TODO: Introduce common unit tests header and move it there
2930
static void set_env(const char *name, const char *value) {
3031
#ifdef _WIN32
31-
(void)_putenv_s(name, value);
32+
(void)_putenv_s(name, value ? value : "");
3233
#else
33-
(void)setenv(name, value, /*overwrite*/ 1);
34+
if (value)
35+
(void)setenv(name, value, /*overwrite*/ 1);
36+
else
37+
(void)unsetenv(name);
3438
#endif
3539
}
3640

@@ -88,12 +92,47 @@ class PersistenDeviceCodeCache : public ::testing::Test {
8892
return _putenv_s(name, value);
8993
}
9094
#endif
95+
96+
std::optional<std::string> SYCLCachePersistentBefore;
97+
bool SYCLCachePersistentChanged = false;
98+
99+
// Caches the initial value of the SYCL_CACHE_PERSISTENT environment variable
100+
// before overwriting it with the new value.
101+
// Tear-down will reset the environment variable.
102+
void SetSYCLCachePersistentEnv(const char *NewValue) {
103+
char *SYCLCachePersistent = getenv("SYCL_CACHE_PERSISTENT");
104+
// We can skip if the new value is the same as the old one.
105+
if ((!NewValue && !SYCLCachePersistent) ||
106+
(NewValue && SYCLCachePersistent &&
107+
!strcmp(NewValue, SYCLCachePersistent)))
108+
return;
109+
110+
// Cache the old value of SYCL_CACHE_PERSISTENT if it is not already saved.
111+
if (!SYCLCachePersistentChanged && SYCLCachePersistent)
112+
SYCLCachePersistentBefore = std::string{SYCLCachePersistent};
113+
114+
// Set the environment variable and signal the configuration file and the
115+
// persistent cache.
116+
set_env("SYCL_CACHE_PERSISTENT", NewValue);
117+
sycl::detail::SYCLConfig<sycl::detail::SYCL_CACHE_PERSISTENT>::reset();
118+
detail::PersistentDeviceCodeCache::reparseConfig();
119+
SYCLCachePersistentChanged = true;
120+
}
121+
91122
virtual void SetUp() {
92123
EXPECT_NE(getenv("SYCL_CACHE_DIR"), nullptr)
93124
<< "Please set SYCL_CACHE_DIR environment variable pointing to cache "
94125
"location.";
95126
}
96127

128+
virtual void TearDown() {
129+
// If we changed the cache, set it back to the old value.
130+
if (SYCLCachePersistentChanged)
131+
SetSYCLCachePersistentEnv(SYCLCachePersistentBefore
132+
? SYCLCachePersistentBefore->c_str()
133+
: nullptr);
134+
}
135+
97136
PersistenDeviceCodeCache() : Plt{default_selector()} {
98137

99138
if (Plt.is_host() || Plt.get_backend() != backend::opencl) {
@@ -119,8 +158,7 @@ class PersistenDeviceCodeCache : public ::testing::Test {
119158
return;
120159
}
121160

122-
set_env("SYCL_CACHE_PERSISTENT", "1");
123-
sycl::detail::SYCLConfig<sycl::detail::SYCL_CACHE_PERSISTENT>::reset();
161+
SetSYCLCachePersistentEnv("1");
124162

125163
std::string BuildOptions{"--concurrent-access=" +
126164
std::to_string(ThreadCount)};
@@ -188,8 +226,7 @@ TEST_F(PersistenDeviceCodeCache, KeysWithNullTermSymbol) {
188226
return;
189227
}
190228

191-
set_env("SYCL_CACHE_PERSISTENT", "1");
192-
sycl::detail::SYCLConfig<sycl::detail::SYCL_CACHE_PERSISTENT>::reset();
229+
SetSYCLCachePersistentEnv("1");
193230

194231
std::string Key{'1', '\0', '3', '4', '\0'};
195232
std::vector<unsigned char> SpecConst(Key.begin(), Key.end());
@@ -247,8 +284,7 @@ TEST_F(PersistenDeviceCodeCache, CorruptedCacheFiles) {
247284
return;
248285
}
249286

250-
set_env("SYCL_CACHE_PERSISTENT", "1");
251-
sycl::detail::SYCLConfig<sycl::detail::SYCL_CACHE_PERSISTENT>::reset();
287+
SetSYCLCachePersistentEnv("1");
252288

253289
std::string BuildOptions{"--corrupted-file"};
254290
std::string ItemDir = detail::PersistentDeviceCodeCache::getCacheItemPath(
@@ -318,8 +354,7 @@ TEST_F(PersistenDeviceCodeCache, LockFile) {
318354
return;
319355
}
320356

321-
set_env("SYCL_CACHE_PERSISTENT", "1");
322-
sycl::detail::SYCLConfig<sycl::detail::SYCL_CACHE_PERSISTENT>::reset();
357+
SetSYCLCachePersistentEnv("1");
323358

324359
std::string BuildOptions{"--obsolete-lock"};
325360
std::string ItemDir = detail::PersistentDeviceCodeCache::getCacheItemPath(
@@ -375,8 +410,7 @@ TEST_F(PersistenDeviceCodeCache, AccessDeniedForCacheDir) {
375410
return;
376411
}
377412

378-
set_env("SYCL_CACHE_PERSISTENT", "1");
379-
sycl::detail::SYCLConfig<sycl::detail::SYCL_CACHE_PERSISTENT>::reset();
413+
SetSYCLCachePersistentEnv("1");
380414

381415
std::string BuildOptions{"--build-options"};
382416
std::string ItemDir = detail::PersistentDeviceCodeCache::getCacheItemPath(

0 commit comments

Comments
 (0)