From 7eb81e9d8584f5f82437be1d889348bfe92ac5cd Mon Sep 17 00:00:00 2001 From: Filip Hazubski Date: Wed, 26 May 2021 13:51:11 +0000 Subject: [PATCH] Unify StreamProperties structs Introduce functions allowing to copy values from one struct to another, while correctly setting values of isDirty field. Signed-off-by: Filip Hazubski --- level_zero/core/source/cmdlist/cmdlist_hw.inl | 12 +-- .../core/source/cmdqueue/cmdqueue_hw.inl | 11 +-- .../encode_compute_mode_tgllp_plus.inl | 2 +- .../command_stream_receiver_hw_base.inl | 2 +- .../definitions/stream_properties.inl | 7 ++ .../command_stream/stream_properties.cpp | 35 ++++++--- .../source/command_stream/stream_properties.h | 6 -- shared/source/helpers/hw_helper.h | 8 -- shared/source/helpers/hw_helper_bdw_plus.inl | 5 -- .../stream_properties_tests.cpp | 4 + .../stream_properties_tests_common.cpp | 76 ++++++++++++++++--- .../stream_properties_tests_common.h | 2 + 12 files changed, 117 insertions(+), 53 deletions(-) diff --git a/level_zero/core/source/cmdlist/cmdlist_hw.inl b/level_zero/core/source/cmdlist/cmdlist_hw.inl index 9823ac78e2..1583768ead 100644 --- a/level_zero/core/source/cmdlist/cmdlist_hw.inl +++ b/level_zero/core/source/cmdlist/cmdlist_hw.inl @@ -1852,14 +1852,14 @@ void CommandListCoreFamily::updateStreamProperties(Kernel &kernel using VFE_STATE_TYPE = typename GfxFamily::VFE_STATE_TYPE; if (!containsAnyKernel) { - requiredStreamState.setCooperativeKernelProperties(kernel.usesSyncBuffer(), device->getHwInfo()); + requiredStreamState.frontEndState.setProperties(kernel.usesSyncBuffer(), device->getHwInfo()); finalStreamState = requiredStreamState; containsAnyKernel = true; } auto &hwInfo = device->getHwInfo(); - auto programVfe = finalStreamState.setCooperativeKernelProperties(kernel.usesSyncBuffer(), hwInfo); - if (programVfe) { + finalStreamState.frontEndState.setProperties(kernel.usesSyncBuffer(), hwInfo); + if (finalStreamState.frontEndState.isDirty()) { auto pVfeStateAddress = NEO::PreambleHelper::getSpaceForVfeState(commandContainer.getCommandStream(), hwInfo, engineGroupType); auto pVfeState = new VFE_STATE_TYPE; NEO::PreambleHelper::programVfeState(pVfeState, hwInfo, 0, 0, device->getMaxNumHwThreads(), @@ -1869,9 +1869,9 @@ void CommandListCoreFamily::updateStreamProperties(Kernel &kernel auto &kernelAttributes = kernel.getKernelDescriptor().kernelAttributes; auto &neoDevice = *device->getNEODevice(); - finalStreamState.setStateComputeModeProperties(false, kernelAttributes.numGrfRequired, isMultiOsContextCapable, - kernelAttributes.flags.useGlobalAtomics, - (neoDevice.getNumAvailableDevices() > 1)); + finalStreamState.stateComputeMode.setProperties(false, kernelAttributes.numGrfRequired, isMultiOsContextCapable, + kernelAttributes.flags.useGlobalAtomics, + (neoDevice.getNumAvailableDevices() > 1)); if (finalStreamState.stateComputeMode.isDirty()) { NEO::EncodeWA::encodeAdditionalPipelineSelect(neoDevice, *commandContainer.getCommandStream(), true); NEO::EncodeComputeMode::adjustComputeMode(*commandContainer.getCommandStream(), nullptr, finalStreamState.stateComputeMode); diff --git a/level_zero/core/source/cmdqueue/cmdqueue_hw.inl b/level_zero/core/source/cmdqueue/cmdqueue_hw.inl index 14f285f757..177bcc8d89 100644 --- a/level_zero/core/source/cmdqueue/cmdqueue_hw.inl +++ b/level_zero/core/source/cmdqueue/cmdqueue_hw.inl @@ -330,7 +330,8 @@ ze_result_t CommandQueueHw::executeCommandLists( if (!isCopyOnlyCommandQueue) { auto &requiredStreamState = commandList->getRequiredStreamState(); - auto programVfe = streamProperties.setCooperativeKernelProperties(requiredStreamState.getCooperativeKernelProperties(), hwInfo); + streamProperties.frontEndState.setProperties(requiredStreamState.frontEndState); + auto programVfe = streamProperties.frontEndState.isDirty(); if (frontEndStateDirty) { programVfe = true; frontEndStateDirty = false; @@ -339,7 +340,7 @@ ze_result_t CommandQueueHw::executeCommandLists( programFrontEnd(scratchSpaceController->getScratchPatchAddress(), scratchSpaceController->getPerThreadScratchSpaceSize(), child); } auto &finalStreamState = commandList->getFinalStreamState(); - streamProperties.setCooperativeKernelProperties(finalStreamState.getCooperativeKernelProperties(), hwInfo); + streamProperties.frontEndState.setProperties(finalStreamState.frontEndState); } patchCommands(*commandList, scratchSpaceController->getScratchPatchAddress()); @@ -462,13 +463,13 @@ size_t CommandQueueHw::estimateFrontEndCmdSizeForMultipleCommandL auto streamPropertiesCopy = streamProperties; auto singleFrontEndCmdSize = estimateFrontEndCmdSize(); - auto &hwInfo = device->getHwInfo(); size_t estimatedSize = 0; for (size_t i = 0; i < numCommandLists; i++) { auto commandList = CommandList::fromHandle(phCommandLists[i]); auto &requiredStreamState = commandList->getRequiredStreamState(); - auto isVfeRequired = streamPropertiesCopy.setCooperativeKernelProperties(requiredStreamState.getCooperativeKernelProperties(), hwInfo); + streamPropertiesCopy.frontEndState.setProperties(requiredStreamState.frontEndState); + auto isVfeRequired = streamPropertiesCopy.frontEndState.isDirty(); if (isFrontEndStateDirty) { isVfeRequired = true; isFrontEndStateDirty = false; @@ -477,7 +478,7 @@ size_t CommandQueueHw::estimateFrontEndCmdSizeForMultipleCommandL estimatedSize += singleFrontEndCmdSize; } auto &finalStreamState = commandList->getFinalStreamState(); - streamPropertiesCopy.setCooperativeKernelProperties(finalStreamState.getCooperativeKernelProperties(), hwInfo); + streamPropertiesCopy.frontEndState.setProperties(finalStreamState.frontEndState); } return estimatedSize; diff --git a/shared/source/command_container/encode_compute_mode_tgllp_plus.inl b/shared/source/command_container/encode_compute_mode_tgllp_plus.inl index 3573547215..ae62fa9ee3 100644 --- a/shared/source/command_container/encode_compute_mode_tgllp_plus.inl +++ b/shared/source/command_container/encode_compute_mode_tgllp_plus.inl @@ -14,7 +14,7 @@ template void EncodeStates::adjustStateComputeMode(LinearStream &csr, uint32_t numGrfRequired, void *const stateComputeModePtr, bool isMultiOsContextCapable, bool requiresCoherency, bool useGlobalAtomics, bool areMultipleSubDevicesInContext) { StreamProperties properties{}; - properties.setStateComputeModeProperties(requiresCoherency, numGrfRequired, isMultiOsContextCapable, useGlobalAtomics, areMultipleSubDevicesInContext); + properties.stateComputeMode.setProperties(requiresCoherency, numGrfRequired, isMultiOsContextCapable, useGlobalAtomics, areMultipleSubDevicesInContext); EncodeComputeMode::adjustComputeMode(csr, stateComputeModePtr, properties.stateComputeMode); } diff --git a/shared/source/command_stream/command_stream_receiver_hw_base.inl b/shared/source/command_stream/command_stream_receiver_hw_base.inl index 5fc4b044e9..0c7afdff9e 100644 --- a/shared/source/command_stream/command_stream_receiver_hw_base.inl +++ b/shared/source/command_stream/command_stream_receiver_hw_base.inl @@ -913,7 +913,7 @@ inline void CommandStreamReceiverHw::programVFEState(LinearStream &cs auto engineGroupType = hwHelper.getEngineGroupType(getOsContext().getEngineType(), hwInfo); auto pVfeState = PreambleHelper::getSpaceForVfeState(&csr, hwInfo, engineGroupType); StreamProperties streamProperties{}; - streamProperties.setCooperativeKernelProperties(lastKernelExecutionType == KernelExecutionType::Concurrent, hwInfo); + streamProperties.frontEndState.setProperties(lastKernelExecutionType == KernelExecutionType::Concurrent, hwInfo); PreambleHelper::programVfeState( pVfeState, hwInfo, requiredScratchSize, getScratchPatchAddress(), maxFrontEndThreads, lastAdditionalKernelExecInfo, streamProperties); diff --git a/shared/source/command_stream/definitions/stream_properties.inl b/shared/source/command_stream/definitions/stream_properties.inl index 9b85cf7583..4dcf8043c2 100644 --- a/shared/source/command_stream/definitions/stream_properties.inl +++ b/shared/source/command_stream/definitions/stream_properties.inl @@ -12,11 +12,18 @@ namespace NEO { struct StateComputeModeProperties { StreamProperty isCoherencyRequired{}; + void setProperties(bool requiresCoherency, uint32_t numGrfRequired, bool isMultiOsContextCapable, + bool useGlobalAtomics, bool areMultipleSubDevicesInContext); + void setProperties(const StateComputeModeProperties &properties); bool isDirty(); void clearIsDirty(); }; struct FrontEndProperties { + void setProperties(bool isCooperativeKernel, const HardwareInfo &hwInfo); + void setProperties(const FrontEndProperties &properties); + bool isDirty(); + void clearIsDirty(); }; } // namespace NEO diff --git a/shared/source/command_stream/stream_properties.cpp b/shared/source/command_stream/stream_properties.cpp index 7a36c64c53..08dada2660 100644 --- a/shared/source/command_stream/stream_properties.cpp +++ b/shared/source/command_stream/stream_properties.cpp @@ -9,20 +9,18 @@ using namespace NEO; -bool StreamProperties::setCooperativeKernelProperties(int32_t cooperativeKernelProperties, const HardwareInfo &hwInfo) { - return false; -} - -int32_t StreamProperties::getCooperativeKernelProperties() const { - return -1; -} - -void StreamProperties::setStateComputeModeProperties(bool requiresCoherency, uint32_t numGrfRequired, bool isMultiOsContextCapable, - bool useGlobalAtomics, bool areMultipleSubDevicesInContext) { - stateComputeMode.clearIsDirty(); +void StateComputeModeProperties::setProperties(bool requiresCoherency, uint32_t numGrfRequired, bool isMultiOsContextCapable, + bool useGlobalAtomics, bool areMultipleSubDevicesInContext) { + clearIsDirty(); int32_t isCoherencyRequired = (requiresCoherency ? 1 : 0); - stateComputeMode.isCoherencyRequired.set(isCoherencyRequired); + this->isCoherencyRequired.set(isCoherencyRequired); +} + +void StateComputeModeProperties::setProperties(const StateComputeModeProperties &properties) { + clearIsDirty(); + + isCoherencyRequired.set(properties.isCoherencyRequired.value); } bool StateComputeModeProperties::isDirty() { @@ -32,3 +30,16 @@ bool StateComputeModeProperties::isDirty() { void StateComputeModeProperties::clearIsDirty() { isCoherencyRequired.isDirty = false; } + +void FrontEndProperties::setProperties(bool isCooperativeKernel, const HardwareInfo &hwInfo) { +} + +void FrontEndProperties::setProperties(const FrontEndProperties &properties) { +} + +bool FrontEndProperties::isDirty() { + return false; +} + +void FrontEndProperties::clearIsDirty() { +} diff --git a/shared/source/command_stream/stream_properties.h b/shared/source/command_stream/stream_properties.h index 51e9c5a5e1..ab9aa7c570 100644 --- a/shared/source/command_stream/stream_properties.h +++ b/shared/source/command_stream/stream_properties.h @@ -15,12 +15,6 @@ namespace NEO { struct StreamProperties { - bool setCooperativeKernelProperties(int32_t cooperativeKernelProperties, const HardwareInfo &hwInfo); - int32_t getCooperativeKernelProperties() const; - - void setStateComputeModeProperties(bool requiresCoherency, uint32_t numGrfRequired, bool isMultiOsContextCapable, - bool useGlobalAtomics, bool areMultipleSubDevicesInContext); - StateComputeModeProperties stateComputeMode{}; FrontEndProperties frontEndState{}; }; diff --git a/shared/source/helpers/hw_helper.h b/shared/source/helpers/hw_helper.h index b489c3b874..5fbaeb5582 100644 --- a/shared/source/helpers/hw_helper.h +++ b/shared/source/helpers/hw_helper.h @@ -42,11 +42,6 @@ enum class LocalMemoryAccessMode { CpuAccessDisallowed = 3 }; -enum class FrontEndType { - Video, - Compute -}; - class HwHelper { public: using EngineInstancesContainer = StackVec; @@ -74,7 +69,6 @@ class HwHelper { virtual bool preferSmallWorkgroupSizeForKernel(const size_t size, const HardwareInfo &hwInfo) const = 0; virtual bool isBufferSizeSuitableForRenderCompression(const size_t size) const = 0; virtual bool obtainBlitterPreference(const HardwareInfo &hwInfo) const = 0; - virtual FrontEndType getFrontEndType(const HardwareInfo &hwInfo) const = 0; virtual bool checkResourceCompatibility(GraphicsAllocation &graphicsAllocation) = 0; virtual bool allowRenderCompression(const HardwareInfo &hwInfo) const = 0; virtual bool isBlitCopyRequiredForLocalMemory(const HardwareInfo &hwInfo, const GraphicsAllocation &allocation) const = 0; @@ -244,8 +238,6 @@ class HwHelperHw : public HwHelper { bool obtainBlitterPreference(const HardwareInfo &hwInfo) const override; - FrontEndType getFrontEndType(const HardwareInfo &hwInfo) const override; - bool checkResourceCompatibility(GraphicsAllocation &graphicsAllocation) override; bool timestampPacketWriteSupported() const override; diff --git a/shared/source/helpers/hw_helper_bdw_plus.inl b/shared/source/helpers/hw_helper_bdw_plus.inl index 8055e62c3d..36e5bcb187 100644 --- a/shared/source/helpers/hw_helper_bdw_plus.inl +++ b/shared/source/helpers/hw_helper_bdw_plus.inl @@ -55,11 +55,6 @@ bool HwHelperHw::obtainBlitterPreference(const HardwareInfo &hwInfo) con return false; } -template -FrontEndType HwHelperHw::getFrontEndType(const HardwareInfo &hwInfo) const { - return FrontEndType::Video; -} - template const HwHelper::EngineInstancesContainer HwHelperHw::getGpgpuEngineInstances(const HardwareInfo &hwInfo) const { return { diff --git a/shared/test/unit_test/command_stream/stream_properties_tests.cpp b/shared/test/unit_test/command_stream/stream_properties_tests.cpp index 63d88d1b94..610c6d0e4f 100644 --- a/shared/test/unit_test/command_stream/stream_properties_tests.cpp +++ b/shared/test/unit_test/command_stream/stream_properties_tests.cpp @@ -16,4 +16,8 @@ std::vector getAllStateComputeModeProperties(StateComputeModeP return allProperties; } +std::vector getAllFrontEndProperties(FrontEndProperties &properties) { + return {}; +} + } // namespace NEO diff --git a/shared/test/unit_test/command_stream/stream_properties_tests_common.cpp b/shared/test/unit_test/command_stream/stream_properties_tests_common.cpp index f9c8870f53..38225950b5 100644 --- a/shared/test/unit_test/command_stream/stream_properties_tests_common.cpp +++ b/shared/test/unit_test/command_stream/stream_properties_tests_common.cpp @@ -44,32 +44,90 @@ TEST(StreamPropertiesTests, whenPropertyValueIsChangedThenProperStateIsSet) { TEST(StreamPropertiesTests, whenSettingStateComputeModePropertiesThenCorrectValuesAreSet) { StreamProperties properties; for (auto requiresCoherency : ::testing::Bool()) { - properties.setStateComputeModeProperties(requiresCoherency, 0u, false, false, false); + properties.stateComputeMode.setProperties(requiresCoherency, 0u, false, false, false); EXPECT_EQ(requiresCoherency, properties.stateComputeMode.isCoherencyRequired.value); } } -TEST(StreamPropertiesTests, givenVariousStatesOfThePropertiesWhenIsStateComputeModeDirtyIsQueriedThenCorrectValueIsReturned) { - struct MockStateComputeModeProperties : StateComputeModeProperties { - using StateComputeModeProperties::clearIsDirty; +template +using getAllPropertiesFunctionPtr = std::vector (*)(PropertiesT &properties); + +template getAllProperties> +void verifyIsDirty() { + struct MockPropertiesT : PropertiesT { + using PropertiesT::clearIsDirty; }; - MockStateComputeModeProperties properties; + MockPropertiesT properties; + auto allProperties = getAllProperties(properties); EXPECT_FALSE(properties.isDirty()); - for (auto pProperty : getAllStateComputeModeProperties(properties)) { + for (auto pProperty : allProperties) { pProperty->isDirty = true; EXPECT_TRUE(properties.isDirty()); pProperty->isDirty = false; EXPECT_FALSE(properties.isDirty()); } - for (auto pProperty : getAllStateComputeModeProperties(properties)) { + for (auto pProperty : allProperties) { pProperty->isDirty = true; } - EXPECT_TRUE(properties.isDirty()); + + EXPECT_EQ(!allProperties.empty(), properties.isDirty()); properties.clearIsDirty(); - for (auto pProperty : getAllStateComputeModeProperties(properties)) { + for (auto pProperty : allProperties) { EXPECT_FALSE(pProperty->isDirty); } EXPECT_FALSE(properties.isDirty()); } + +TEST(StreamPropertiesTests, givenVariousStatesOfStateComputeModePropertiesWhenIsDirtyIsQueriedThenCorrectValueIsReturned) { + verifyIsDirty(); +} + +TEST(StreamPropertiesTests, givenVariousStatesOfFrontEndPropertiesWhenIsDirtyIsQueriedThenCorrectValueIsReturned) { + verifyIsDirty(); +} + +template getAllProperties> +void verifySettingPropertiesFromOtherStruct() { + PropertiesT propertiesDestination; + PropertiesT propertiesSource; + + auto allPropertiesDestination = getAllProperties(propertiesDestination); + auto allPropertiesSource = getAllProperties(propertiesSource); + + int valueToSet = 1; + for (auto pProperty : allPropertiesSource) { + pProperty->value = valueToSet; + valueToSet++; + } + EXPECT_FALSE(propertiesSource.isDirty()); + + propertiesDestination.setProperties(propertiesSource); + EXPECT_EQ(!allPropertiesDestination.empty(), propertiesDestination.isDirty()); + + int expectedValue = 1; + for (auto pProperty : allPropertiesDestination) { + EXPECT_EQ(expectedValue, pProperty->value); + EXPECT_TRUE(pProperty->isDirty); + expectedValue++; + } + + propertiesDestination.setProperties(propertiesSource); + EXPECT_FALSE(propertiesDestination.isDirty()); + + expectedValue = 1; + for (auto pProperty : allPropertiesDestination) { + EXPECT_EQ(expectedValue, pProperty->value); + EXPECT_FALSE(pProperty->isDirty); + expectedValue++; + } +} + +TEST(StreamPropertiesTests, givenOtherStateComputeModePropertiesStructWhenSetPropertiesIsCalledThenCorrectValuesAreSet) { + verifySettingPropertiesFromOtherStruct(); +} + +TEST(StreamPropertiesTests, givenOtherFrontEndPropertiesStructWhenSetPropertiesIsCalledThenCorrectValuesAreSet) { + verifySettingPropertiesFromOtherStruct(); +} diff --git a/shared/test/unit_test/command_stream/stream_properties_tests_common.h b/shared/test/unit_test/command_stream/stream_properties_tests_common.h index c6f748b4f3..a877e73caa 100644 --- a/shared/test/unit_test/command_stream/stream_properties_tests_common.h +++ b/shared/test/unit_test/command_stream/stream_properties_tests_common.h @@ -11,9 +11,11 @@ namespace NEO { +struct FrontEndProperties; struct StateComputeModeProperties; struct StreamProperty; std::vector getAllStateComputeModeProperties(StateComputeModeProperties &properties); +std::vector getAllFrontEndProperties(FrontEndProperties &properties); } // namespace NEO