Skip to content

Commit 2aa4798

Browse files
committed
as discussed, moving clearing of platform cache to beginning of mock. Also fix the otherwise tautological DeviceRefCounter test.
Signed-off-by: Chris Perkins <[email protected]>
1 parent 0c95213 commit 2aa4798

File tree

2 files changed

+21
-17
lines changed

2 files changed

+21
-17
lines changed

sycl/unittests/context_device/DeviceRefCounter.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
66
//
77
//===----------------------------------------------------------------------===//
8+
#include <detail/global_handler.hpp>
89
#include <gtest/gtest.h>
910
#include <helpers/UrMock.hpp>
1011
#include <sycl/sycl.hpp>
@@ -28,12 +29,10 @@ static ur_result_t redefinedDeviceReleaseAfter(void *) {
2829
return UR_RESULT_SUCCESS;
2930
}
3031

31-
#ifndef WIN32
32-
// This test passes because the UrMock emulates teardown on Linux, but
33-
// on Windows there is a difference so this test is skipped.
3432
TEST(DevRefCounter, DevRefCounter) {
3533
{
3634
sycl::unittest::UrMock<> Mock;
35+
EXPECT_EQ(DevRefCounter, 0);
3736

3837
mock::getCallbacks().set_after_callback("urDeviceGet",
3938
&redefinedDevicesGetAfter);
@@ -44,7 +43,12 @@ TEST(DevRefCounter, DevRefCounter) {
4443
sycl::platform Plt = sycl::platform();
4544

4645
Plt.get_devices();
47-
} // <- ~UrMock destructor called here.
46+
EXPECT_NE(DevRefCounter, 0);
47+
// This is the behavior that SYCL performs at shutdown, but there
48+
// are timing differences Lin/Win and shared/static that make
49+
// it not map correctly into our mock.
50+
// So for this test, we just do it.
51+
sycl::detail::GlobalHandler::instance().getPlatformCache().clear();
52+
}
4853
EXPECT_EQ(DevRefCounter, 0);
4954
}
50-
#endif // !WIN32

sycl/unittests/helpers/UrMock.hpp

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,16 @@ template <sycl::backend Backend = backend::opencl> class UrMock {
596596

597597
sycl::detail::ur::initializeUr(UrLoaderConfig);
598598
urLoaderConfigRelease(UrLoaderConfig);
599+
600+
// We clear platform cache for each test run, so that tests can have a
601+
// different backend. This forces platforms to be reconstructed (and thus
602+
// queries about UR backend info to be called again) This also erases each
603+
// platform's devices (normally done in the library shutdown) so that
604+
// platforms/devices' lifetimes could work in unittests scenario. In SYCL,
605+
// this is normally done at shutdown, but between Win/Lin and static/shared
606+
// differences, there is no correct parallel in the ~UrMock destructor.
607+
// Instead we do it here. Simple and clean.
608+
detail::GlobalHandler::instance().getPlatformCache().clear();
599609
}
600610

601611
UrMock(UrMock<Backend> &&Other) = delete;
@@ -606,18 +616,8 @@ template <sycl::backend Backend = backend::opencl> class UrMock {
606616
// these between tests
607617
detail::GlobalHandler::instance().prepareSchedulerToRelease(true);
608618
detail::GlobalHandler::instance().releaseDefaultContexts();
609-
#ifndef WIN32
610-
// Clear platform cache in case subsequent tests want a different backend.
611-
// This forces platforms to be reconstructed (and thus queries about UR
612-
// backend info to be called again) This also erases each platform's devices
613-
// (normally done in the library shutdown) so that platforms/devices'
614-
// lifetimes could work in unittests scenario. But on Windows, because the
615-
// shutdown/teardown timing is slightly different, doing this now can cause
616-
// a race which will cause errors. At the moment, skipping this on Windows
617-
// is fine. If, in the future, we really need to clear this, do it in the
618-
// UrMock constructor.
619-
detail::GlobalHandler::instance().getPlatformCache().clear();
620-
#endif // !WIN32
619+
// the platform cache is cleared at the BEGINING of the mock.
620+
621621
mock::getCallbacks().resetCallbacks();
622622
}
623623

0 commit comments

Comments
 (0)