From f2c11eb87003d5c138f2362c3f372f01fb6c2dca Mon Sep 17 00:00:00 2001 From: Filip Hazubski Date: Thu, 2 Apr 2020 16:38:42 +0200 Subject: [PATCH] Update sub device internal ref counts Change-Id: I82eea99bbb3d1edc32d09c0b703dee30b62f6b76 Signed-off-by: Filip Hazubski --- opencl/source/cl_device/cl_device.cpp | 17 ++++++ opencl/source/cl_device/cl_device.h | 3 + .../sync_buffer_handler_tests.cpp | 18 +++--- .../unit_test/device/sub_device_tests.cpp | 60 ++++++++++++++++++- shared/source/device/device.h | 7 +++ shared/source/device/root_device.cpp | 3 +- shared/source/device/sub_device.cpp | 7 +++ shared/source/device/sub_device.h | 3 + 8 files changed, 107 insertions(+), 11 deletions(-) diff --git a/opencl/source/cl_device/cl_device.cpp b/opencl/source/cl_device/cl_device.cpp index 7b77c2c4bd..6f62ebf182 100644 --- a/opencl/source/cl_device/cl_device.cpp +++ b/opencl/source/cl_device/cl_device.cpp @@ -69,6 +69,23 @@ ClDevice::~ClDevice() { device.decRefInternal(); } +void ClDevice::incRefInternal() { + if (deviceInfo.parentDevice == nullptr) { + BaseObject<_cl_device_id>::incRefInternal(); + return; + } + auto pParentDevice = static_cast(deviceInfo.parentDevice); + pParentDevice->incRefInternal(); +} + +unique_ptr_if_unused ClDevice::decRefInternal() { + if (deviceInfo.parentDevice == nullptr) { + return BaseObject<_cl_device_id>::decRefInternal(); + } + auto pParentDevice = static_cast(deviceInfo.parentDevice); + return pParentDevice->decRefInternal(); +} + void ClDevice::allocateSyncBufferHandler() { TakeOwnershipWrapper lock(*this); if (syncBufferHandler.get() == nullptr) { diff --git a/opencl/source/cl_device/cl_device.h b/opencl/source/cl_device/cl_device.h index def0a19fa4..f05c3a51a6 100644 --- a/opencl/source/cl_device/cl_device.h +++ b/opencl/source/cl_device/cl_device.h @@ -51,6 +51,9 @@ class ClDevice : public BaseObject<_cl_device_id> { explicit ClDevice(Device &device, Platform *platformId); ~ClDevice() override; + void incRefInternal(); + unique_ptr_if_unused decRefInternal(); + unsigned int getEnabledClVersion() const { return enabledClVersion; }; unsigned int getSupportedClVersion() const; diff --git a/opencl/test/unit_test/command_queue/sync_buffer_handler_tests.cpp b/opencl/test/unit_test/command_queue/sync_buffer_handler_tests.cpp index 4a5d994c7f..6496d0258e 100644 --- a/opencl/test/unit_test/command_queue/sync_buffer_handler_tests.cpp +++ b/opencl/test/unit_test/command_queue/sync_buffer_handler_tests.cpp @@ -6,6 +6,7 @@ */ #include "shared/source/program/sync_buffer_handler.h" +#include "shared/test/unit_test/helpers/debug_manager_state_restore.h" #include "opencl/source/api/api.h" #include "opencl/test/unit_test/fixtures/enqueue_handler_fixture.h" @@ -171,17 +172,20 @@ TEST(SyncBufferHandlerDeviceTest, GivenRootDeviceWhenAllocateSyncBufferIsCalledT } TEST(SyncBufferHandlerDeviceTest, GivenSubDeviceWhenAllocateSyncBufferIsCalledTwiceThenTheObjectIsCreatedOnlyOnce) { - const size_t testUsedBufferSize = 100; - MockClDevice rootDevice{new MockDevice}; - ClDevice subDevice{*rootDevice.createSubDevice(0), platform()}; - subDevice.allocateSyncBufferHandler(); - auto syncBufferHandler = reinterpret_cast(subDevice.syncBufferHandler.get()); + DebugManagerStateRestore restorer; + DebugManager.flags.CreateMultipleSubDevices.set(2); + VariableBackup mockDeviceFlagBackup(&MockDevice::createSingleDevice, false); + auto rootDevice = std::make_unique(MockDevice::createWithNewExecutionEnvironment(defaultHwInfo.get())); + auto &subDevice = rootDevice->subDevices[0]; + subDevice->allocateSyncBufferHandler(); + auto syncBufferHandler = reinterpret_cast(subDevice->syncBufferHandler.get()); + const size_t testUsedBufferSize = 100; ASSERT_NE(syncBufferHandler->usedBufferSize, testUsedBufferSize); syncBufferHandler->usedBufferSize = testUsedBufferSize; - subDevice.allocateSyncBufferHandler(); - syncBufferHandler = reinterpret_cast(subDevice.syncBufferHandler.get()); + subDevice->allocateSyncBufferHandler(); + syncBufferHandler = reinterpret_cast(subDevice->syncBufferHandler.get()); EXPECT_EQ(testUsedBufferSize, syncBufferHandler->usedBufferSize); } diff --git a/opencl/test/unit_test/device/sub_device_tests.cpp b/opencl/test/unit_test/device/sub_device_tests.cpp index bc1078d9ef..f31e39ba93 100644 --- a/opencl/test/unit_test/device/sub_device_tests.cpp +++ b/opencl/test/unit_test/device/sub_device_tests.cpp @@ -10,9 +10,9 @@ #include "shared/test/unit_test/helpers/debug_manager_state_restore.h" #include "shared/test/unit_test/helpers/ult_hw_config.h" #include "shared/test/unit_test/helpers/variable_backup.h" -#include "shared/test/unit_test/mocks/mock_device.h" #include "opencl/source/cl_device/cl_device.h" +#include "opencl/test/unit_test/mocks/mock_cl_device.h" #include "opencl/test/unit_test/mocks/mock_memory_manager.h" #include "opencl/test/unit_test/mocks/mock_platform.h" @@ -54,7 +54,7 @@ TEST(SubDevicesTest, givenCreateMultipleSubDevicesFlagSetWhenCreateRootDeviceThe EXPECT_EQ(1u, device->subdevices.at(1)->getNumAvailableDevices()); } -TEST(SubDevicesTest, givenDeviceWithSubDevicesWhenSubDeviceRefcountsAreChangedThenChangeIsPropagatedToRootDevice) { +TEST(SubDevicesTest, givenDeviceWithSubDevicesWhenSubDeviceApiRefCountsAreChangedThenChangeIsPropagatedToRootDevice) { DebugManagerStateRestore restorer; DebugManager.flags.CreateMultipleSubDevices.set(2); VariableBackup mockDeviceFlagBackup(&MockDevice::createSingleDevice, false); @@ -89,6 +89,62 @@ TEST(SubDevicesTest, givenDeviceWithSubDevicesWhenSubDeviceRefcountsAreChangedTh EXPECT_EQ(baseDefaultDeviceInternalRefCount, defaultDevice->getRefInternalCount()); } +TEST(SubDevicesTest, givenDeviceWithSubDevicesWhenSubDeviceInternalRefCountsAreChangedThenChangeIsPropagatedToRootDevice) { + DebugManagerStateRestore restorer; + DebugManager.flags.CreateMultipleSubDevices.set(2); + VariableBackup mockDeviceFlagBackup(&MockDevice::createSingleDevice, false); + auto device = std::unique_ptr(MockDevice::createWithNewExecutionEnvironment(defaultHwInfo.get())); + device->incRefInternal(); + auto subDevice = device->getDeviceById(0); + + auto baseDeviceInternalRefCount = device->getRefInternalCount(); + auto baseSubDeviceInternalRefCount = subDevice->getRefInternalCount(); + + subDevice->incRefInternal(); + EXPECT_EQ(baseDeviceInternalRefCount + 1, device->getRefInternalCount()); + EXPECT_EQ(baseSubDeviceInternalRefCount, subDevice->getRefInternalCount()); + + device->incRefInternal(); + EXPECT_EQ(baseDeviceInternalRefCount + 2, device->getRefInternalCount()); + EXPECT_EQ(baseSubDeviceInternalRefCount, subDevice->getRefInternalCount()); + + subDevice->decRefInternal(); + EXPECT_EQ(baseDeviceInternalRefCount + 1, device->getRefInternalCount()); + EXPECT_EQ(baseSubDeviceInternalRefCount, subDevice->getRefInternalCount()); + + device->decRefInternal(); + EXPECT_EQ(baseDeviceInternalRefCount, device->getRefInternalCount()); + EXPECT_EQ(baseSubDeviceInternalRefCount, subDevice->getRefInternalCount()); +} + +TEST(SubDevicesTest, givenClDeviceWithSubDevicesWhenSubDeviceInternalRefCountsAreChangedThenChangeIsPropagatedToRootDevice) { + DebugManagerStateRestore restorer; + DebugManager.flags.CreateMultipleSubDevices.set(2); + VariableBackup mockDeviceFlagBackup(&MockDevice::createSingleDevice, false); + auto device = std::make_unique(MockDevice::createWithNewExecutionEnvironment(defaultHwInfo.get())); + device->incRefInternal(); + auto &subDevice = device->subDevices[0]; + + auto baseDeviceInternalRefCount = device->getRefInternalCount(); + auto baseSubDeviceInternalRefCount = subDevice->getRefInternalCount(); + + subDevice->incRefInternal(); + EXPECT_EQ(baseDeviceInternalRefCount + 1, device->getRefInternalCount()); + EXPECT_EQ(baseSubDeviceInternalRefCount, subDevice->getRefInternalCount()); + + device->incRefInternal(); + EXPECT_EQ(baseDeviceInternalRefCount + 2, device->getRefInternalCount()); + EXPECT_EQ(baseSubDeviceInternalRefCount, subDevice->getRefInternalCount()); + + subDevice->decRefInternal(); + EXPECT_EQ(baseDeviceInternalRefCount + 1, device->getRefInternalCount()); + EXPECT_EQ(baseSubDeviceInternalRefCount, subDevice->getRefInternalCount()); + + device->decRefInternal(); + EXPECT_EQ(baseDeviceInternalRefCount, device->getRefInternalCount()); + EXPECT_EQ(baseSubDeviceInternalRefCount, subDevice->getRefInternalCount()); +} + TEST(SubDevicesTest, givenDeviceWithSubDevicesWhenSubDeviceCreationFailThenWholeDeviceIsDestroyed) { DebugManagerStateRestore restorer; DebugManager.flags.CreateMultipleSubDevices.set(10); diff --git a/shared/source/device/device.h b/shared/source/device/device.h index a9799ab6b0..d031c7f9be 100644 --- a/shared/source/device/device.h +++ b/shared/source/device/device.h @@ -30,6 +30,13 @@ class Device : public ReferenceTrackedObject { return createDeviceInternals(device); } + virtual void incRefInternal() { + ReferenceTrackedObject::incRefInternal(); + } + virtual unique_ptr_if_unused decRefInternal() { + return ReferenceTrackedObject::decRefInternal(); + } + bool getDeviceAndHostTimer(uint64_t *deviceTimestamp, uint64_t *hostTimestamp) const; bool getHostTimer(uint64_t *hostTimestamp) const; const HardwareInfo &getHardwareInfo() const; diff --git a/shared/source/device/root_device.cpp b/shared/source/device/root_device.cpp index e15ab1c6c0..9cfd52ecd7 100644 --- a/shared/source/device/root_device.cpp +++ b/shared/source/device/root_device.cpp @@ -22,7 +22,7 @@ RootDevice::RootDevice(ExecutionEnvironment *executionEnvironment, uint32_t root RootDevice::~RootDevice() { for (auto subdevice : subdevices) { if (subdevice) { - subdevice->decRefInternal(); + delete subdevice; } } } @@ -67,7 +67,6 @@ bool RootDevice::createDeviceImpl() { if (!subDevice) { return false; } - subDevice->incRefInternal(); subdevices[i] = subDevice; } auto status = Device::createDeviceImpl(); diff --git a/shared/source/device/sub_device.cpp b/shared/source/device/sub_device.cpp index 120e1b05f4..497c4ab57b 100644 --- a/shared/source/device/sub_device.cpp +++ b/shared/source/device/sub_device.cpp @@ -15,6 +15,13 @@ SubDevice::SubDevice(ExecutionEnvironment *executionEnvironment, uint32_t subDev : Device(executionEnvironment), subDeviceIndex(subDeviceIndex), rootDevice(rootDevice) { } +void SubDevice::incRefInternal() { + rootDevice.incRefInternal(); +} +unique_ptr_if_unused SubDevice::decRefInternal() { + return rootDevice.decRefInternal(); +} + DeviceBitfield SubDevice::getDeviceBitfield() const { DeviceBitfield deviceBitfield; deviceBitfield.set(subDeviceIndex); diff --git a/shared/source/device/sub_device.h b/shared/source/device/sub_device.h index 1264f94325..c4417bb6a1 100644 --- a/shared/source/device/sub_device.h +++ b/shared/source/device/sub_device.h @@ -13,6 +13,9 @@ class RootDevice; class SubDevice : public Device { public: SubDevice(ExecutionEnvironment *executionEnvironment, uint32_t subDeviceIndex, RootDevice &rootDevice); + void incRefInternal() override; + unique_ptr_if_unused decRefInternal() override; + uint32_t getNumAvailableDevices() const override; uint32_t getRootDeviceIndex() const override; Device *getDeviceById(uint32_t deviceId) const override;