Repository navigation
[SYCL][Unit] Fix persistent cache unit tests - #23548
Open
uditagarwal97 wants to merge 1 commit into
Open
uditagarwal97 wants to merge 1 commit into
uditagarwal97 wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Relative cache paths can cause fixture setup to fail because directory creation resolves them against different locations.
1 open issue
What changed in this PR
Improves SYCL persistent-cache unit test isolation and fixes in-memory cache configuration resets.
Changes:
- Uses unique cache directories with ownership-based cleanup.
- Re-enables the multi-device persistent-cache test.
- Re-parses
SYCL_CACHE_IN_MEMwhen resetting its cached value.
| File | Description |
|---|---|
| sycl/unittests/program_manager/MultipleDevsKernelBundle.cpp | Re-enables the cache test with a unique directory. |
| sycl/unittests/kernel-and-program/PersistentDeviceCodeCache.cpp | Isolates cache directories and updates environment handling. |
| sycl/source/detail/config.hpp | Corrects cached configuration reset parsing. |
Review effort: Balanced
Comment on lines
+161
to
+163
| ASSERT_NO_ERROR(llvm::sys::fs::create_directories(RootSYCLCacheDir)); | ||
| ASSERT_NO_ERROR(llvm::sys::fs::createUniqueDirectory( | ||
| RootSYCLCacheDir + "/pdcc", UniqueDir)); |
Contributor
Author
There was a problem hiding this comment.
I think that would be an overkill - we never use a relative SYCL_CACHE_DIR in unit tests, so would not run into this problem.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #18122
Two persistent cache unit tests fail intermittently because separate test processes share one on-disk cache directory and remove each other's files.
MultipleDevsKernelBundleTest.PersistentCache- The test is parameterized, LIT runs both the instances parallelly (for SPIRV and binary device image format respectively). This causes race over the common cache directory. To fix this, the test now creates a unique cache directory per run usingllvm::sys::fs::createUniqueDirectory.PersistentDeviceCodeCache.CorruptedCacheFiles- It's a death test (usesASSERT_DEATH). On Windows, gtest runsa death test by re-executing the binary, and the child re-runs the whole test. See https://google.github.io/googletest/reference/assertions.html#death . The child process inherits the environment variable from parent, so we store the unique cache directory name in an environment variable and pass it over to the child process. This ensures that there's no race between the two parameterized runs of this test and their children. Note that, the parent and child should use the same cache directory for the test to work, earlier we used to hardcode the cache directory path but that was causing a race between two parameterized runs of this test.