From fd9fb3e46b80f4a5c3b3e97e2d8640e1bcd37dbf Mon Sep 17 00:00:00 2001 From: Maciej Dziuban Date: Mon, 12 Apr 2021 16:17:35 +0000 Subject: [PATCH] Remove isInitialized() method from OsContext We can fail early, instead of constructing an invalid object Signed-off-by: Maciej Dziuban --- .../os_interface/os_context_tests.cpp | 1 - .../windows/os_context_win_tests.cpp | 26 +++++++------------ .../windows/os_interface_win_tests.cpp | 1 - .../os_interface/windows/wddm23_tests.cpp | 6 ++--- .../source/memory_manager/memory_manager.cpp | 1 - shared/source/os_interface/os_context.h | 1 - .../os_interface/windows/os_context_win.cpp | 23 +++++++--------- .../os_interface/windows/os_context_win.h | 2 -- 8 files changed, 21 insertions(+), 40 deletions(-) diff --git a/opencl/test/unit_test/os_interface/os_context_tests.cpp b/opencl/test/unit_test/os_interface/os_context_tests.cpp index 0d88e5d05f..0f6d7b6a82 100644 --- a/opencl/test/unit_test/os_interface/os_context_tests.cpp +++ b/opencl/test/unit_test/os_interface/os_context_tests.cpp @@ -13,7 +13,6 @@ using namespace NEO; TEST(OSContext, whenCreatingDefaultOsContextThenExpectInitializedAlways) { OsContext *osContext = OsContext::create(nullptr, 0, 0, EngineTypeUsage{aub_stream::ENGINE_RCS, EngineUsage::Regular}, PreemptionMode::Disabled, false); - EXPECT_TRUE(osContext->isInitialized()); EXPECT_FALSE(osContext->isLowPriority()); EXPECT_FALSE(osContext->isInternalEngine()); EXPECT_FALSE(osContext->isRootDevice()); diff --git a/opencl/test/unit_test/os_interface/windows/os_context_win_tests.cpp b/opencl/test/unit_test/os_interface/windows/os_context_win_tests.cpp index c56cd136fa..875f6d06ca 100644 --- a/opencl/test/unit_test/os_interface/windows/os_context_win_tests.cpp +++ b/opencl/test/unit_test/os_interface/windows/os_context_win_tests.cpp @@ -26,29 +26,23 @@ struct OsContextWinTest : public WddmTestWithMockGdiDll { }; TEST_F(OsContextWinTest, givenWddm20WhenCreatingOsContextThenOsContextIsInitialized) { - osContext = std::make_unique(*osInterface->get()->getWddm(), 0u, 1, engineTypeUsage, preemptionMode, false); - EXPECT_TRUE(osContext->isInitialized()); + EXPECT_NO_THROW(osContext = std::make_unique(*osInterface->get()->getWddm(), 0u, 1, engineTypeUsage, preemptionMode, false)); + EXPECT_NE(nullptr, osContext); } -TEST_F(OsContextWinTest, givenWddm20WhenCreatingWddmContextFailThenOsContextIsNotInitialized) { +TEST_F(OsContextWinTest, givenWddm20WhenCreatingWddmContextFailThenOsContextCreationFails) { wddm->device = INVALID_HANDLE; - - osContext = std::make_unique(*osInterface->get()->getWddm(), 0u, 1, engineTypeUsage, preemptionMode, false); - EXPECT_FALSE(osContext->isInitialized()); + EXPECT_ANY_THROW(osContext = std::make_unique(*osInterface->get()->getWddm(), 0u, 1, engineTypeUsage, preemptionMode, false)); } -TEST_F(OsContextWinTest, givenWddm20WhenCreatingWddmMonitorFenceFailThenOsContextIsNotInitialized) { +TEST_F(OsContextWinTest, givenWddm20WhenCreatingWddmMonitorFenceFailThenOsContextCreationFails) { *getCreateSynchronizationObject2FailCallFcn() = true; - - osContext = std::make_unique(*osInterface->get()->getWddm(), 0u, 1, engineTypeUsage, preemptionMode, false); - EXPECT_FALSE(osContext->isInitialized()); + EXPECT_ANY_THROW(osContext = std::make_unique(*osInterface->get()->getWddm(), 0u, 1, engineTypeUsage, preemptionMode, false)); } -TEST_F(OsContextWinTest, givenWddm20WhenRegisterTrimCallbackFailThenOsContextIsNotInitialized) { +TEST_F(OsContextWinTest, givenWddm20WhenRegisterTrimCallbackFailThenOsContextCreationFails) { *getRegisterTrimNotificationFailCallFcn() = true; - - osContext = std::make_unique(*osInterface->get()->getWddm(), 0u, 1, engineTypeUsage, preemptionMode, false); - EXPECT_FALSE(osContext->isInitialized()); + EXPECT_ANY_THROW(osContext = std::make_unique(*osInterface->get()->getWddm(), 0u, 1, engineTypeUsage, preemptionMode, false)); } TEST_F(OsContextWinTest, givenWddm20WhenRegisterTrimCallbackIsDisabledThenOsContextIsInitialized) { @@ -56,6 +50,6 @@ TEST_F(OsContextWinTest, givenWddm20WhenRegisterTrimCallbackIsDisabledThenOsCont DebugManager.flags.DoNotRegisterTrimCallback.set(true); *getRegisterTrimNotificationFailCallFcn() = true; - osContext = std::make_unique(*osInterface->get()->getWddm(), 0u, 1, engineTypeUsage, preemptionMode, false); - EXPECT_TRUE(osContext->isInitialized()); + EXPECT_NO_THROW(osContext = std::make_unique(*osInterface->get()->getWddm(), 0u, 1, engineTypeUsage, preemptionMode, false)); + EXPECT_NE(nullptr, osContext); } diff --git a/opencl/test/unit_test/os_interface/windows/os_interface_win_tests.cpp b/opencl/test/unit_test/os_interface/windows/os_interface_win_tests.cpp index c5636fe456..964733e901 100644 --- a/opencl/test/unit_test/os_interface/windows/os_interface_win_tests.cpp +++ b/opencl/test/unit_test/os_interface/windows/os_interface_win_tests.cpp @@ -34,7 +34,6 @@ TEST(OsContextTest, givenWddmWhenCreateOsContextAfterInitWddmThenOsContextIsInit auto osContext = std::make_unique(*wddm, 0u, 1, HwHelper::get(defaultHwInfo->platform.eRenderCoreFamily).getGpgpuEngineInstances(*defaultHwInfo)[0], preemptionMode, false); - EXPECT_TRUE(osContext->isInitialized()); EXPECT_EQ(osContext->getWddm(), wddm); EXPECT_EQ(1u, wddm->registerTrimCallbackResult.called); } diff --git a/opencl/test/unit_test/os_interface/windows/wddm23_tests.cpp b/opencl/test/unit_test/os_interface/windows/wddm23_tests.cpp index 0982114b7f..8a44eb29f9 100644 --- a/opencl/test/unit_test/os_interface/windows/wddm23_tests.cpp +++ b/opencl/test/unit_test/os_interface/windows/wddm23_tests.cpp @@ -217,8 +217,7 @@ TEST_F(Wddm23TestsWithoutWddmInit, whenInitCalledThenInitializeNewGdiDDIsAndCall TEST_F(Wddm23TestsWithoutWddmInit, whenCreateHwQueueFailedThenReturnFalseFromInit) { wddmMockInterface->forceCreateHwQueueFail = true; - init(); - EXPECT_FALSE(osContext->isInitialized()); + EXPECT_ANY_THROW(init()); } TEST_F(Wddm23TestsWithoutWddmInit, givenFailureOnGdiInitializationWhenCreatingHwQueueThenReturnFailure) { @@ -229,8 +228,7 @@ TEST_F(Wddm23TestsWithoutWddmInit, givenFailureOnGdiInitializationWhenCreatingHw }; auto myMockGdi = new MyMockGdi(); wddm->resetGdi(myMockGdi); - init(); - EXPECT_FALSE(osContext->isInitialized()); + EXPECT_ANY_THROW(init()); EXPECT_EQ(1u, wddmMockInterface->createHwQueueCalled); EXPECT_FALSE(wddmMockInterface->createHwQueueResult); } diff --git a/shared/source/memory_manager/memory_manager.cpp b/shared/source/memory_manager/memory_manager.cpp index 777e301cc1..f2cd32ff5f 100644 --- a/shared/source/memory_manager/memory_manager.cpp +++ b/shared/source/memory_manager/memory_manager.cpp @@ -258,7 +258,6 @@ OsContext *MemoryManager::createAndRegisterOsContext(CommandStreamReceiver *comm auto contextId = ++latestContextId; auto osContext = OsContext::create(peekExecutionEnvironment().rootDeviceEnvironments[commandStreamReceiver->getRootDeviceIndex()]->osInterface.get(), contextId, deviceBitfield, typeUsage, preemptionMode, rootDevice); - UNRECOVERABLE_IF(!osContext->isInitialized()); osContext->incRefInternal(); registeredEngines.emplace_back(commandStreamReceiver, osContext); diff --git a/shared/source/os_interface/os_context.h b/shared/source/os_interface/os_context.h index 37bbd0bd4d..4a80cfc96d 100644 --- a/shared/source/os_interface/os_context.h +++ b/shared/source/os_interface/os_context.h @@ -35,7 +35,6 @@ class OsContext : public ReferenceTrackedObject { bool isLowPriority() const { return engineUsage == EngineUsage::LowPriority; } bool isInternalEngine() const { return engineUsage == EngineUsage::Internal; } bool isRootDevice() const { return rootDevice; } - virtual bool isInitialized() const { return true; } virtual bool isDirectSubmissionSupported(const HardwareInfo &hwInfo) const { return false; } bool isDefaultContext() const { return defaultContext; } void setDefaultContext(bool value) { defaultContext = value; } diff --git a/shared/source/os_interface/windows/os_context_win.cpp b/shared/source/os_interface/windows/os_context_win.cpp index 14d6055d53..576bc35b2e 100644 --- a/shared/source/os_interface/windows/os_context_win.cpp +++ b/shared/source/os_interface/windows/os_context_win.cpp @@ -28,21 +28,16 @@ OsContextWin::OsContextWin(Wddm &wddm, uint32_t contextId, DeviceBitfield device residencyController(wddm, contextId) { auto wddmInterface = wddm.getWddmInterface(); - if (!wddm.createContext(*this)) { - return; - } - if (wddmInterface->hwQueuesSupported()) { - if (!wddmInterface->createHwQueue(*this)) { - return; - } - } - initialized = wddmInterface->createMonitoredFence(*this); - residencyController.registerCallback(); -}; + UNRECOVERABLE_IF(!wddm.createContext(*this)); -bool OsContextWin::isInitialized() const { - return (initialized && residencyController.isInitialized()); -} + if (wddmInterface->hwQueuesSupported()) { + UNRECOVERABLE_IF(!wddmInterface->createHwQueue(*this)); + } + UNRECOVERABLE_IF(!wddmInterface->createMonitoredFence(*this)); + + residencyController.registerCallback(); + UNRECOVERABLE_IF(!residencyController.isInitialized()); +}; OsContextWin::~OsContextWin() { wddm.getWddmInterface()->destroyHwQueue(hardwareQueue.handle); diff --git a/shared/source/os_interface/windows/os_context_win.h b/shared/source/os_interface/windows/os_context_win.h index 4097d2496b..5c8be779d2 100644 --- a/shared/source/os_interface/windows/os_context_win.h +++ b/shared/source/os_interface/windows/os_context_win.h @@ -32,12 +32,10 @@ class OsContextWin : public OsContext { void setWddmContextHandle(D3DKMT_HANDLE wddmContextHandle) { this->wddmContextHandle = wddmContextHandle; } HardwareQueue getHwQueue() const { return hardwareQueue; } void setHwQueue(HardwareQueue hardwareQueue) { this->hardwareQueue = hardwareQueue; } - bool isInitialized() const override; Wddm *getWddm() const { return &wddm; } MOCKABLE_VIRTUAL WddmResidencyController &getResidencyController() { return residencyController; } protected: - bool initialized = false; D3DKMT_HANDLE wddmContextHandle = 0; HardwareQueue hardwareQueue; Wddm &wddm;