From e36d95e03917c87dd8d4d62f5a0ee5d777ca49d1 Mon Sep 17 00:00:00 2001 From: Bartosz Dunajski Date: Tue, 19 May 2020 16:20:41 +0200 Subject: [PATCH] Add helpers to avoid offsetof usage Change-Id: I8f0ec5240b2ec2bd99b712271d87b88ffed2c5b3 Signed-off-by: Bartosz Dunajski --- opencl/source/command_queue/enqueue_common.h | 5 ++--- .../command_queue/gpgpu_walker_bdw_plus.inl | 2 +- .../command_queue/hardware_interface_base.inl | 2 +- .../command_queue/blit_enqueue_tests.cpp | 12 ++++++------ .../helpers/timestamp_packet_tests.cpp | 19 ++++++++++++++++--- .../test/unit_test/mem_obj/buffer_tests.cpp | 12 ++++++------ .../command_stream_receiver_hw_base.inl | 5 ++--- shared/source/helpers/timestamp_packet.h | 15 +++++++++++---- 8 files changed, 45 insertions(+), 27 deletions(-) diff --git a/opencl/source/command_queue/enqueue_common.h b/opencl/source/command_queue/enqueue_common.h index f19b15b8c3..2866f43a9d 100644 --- a/opencl/source/command_queue/enqueue_common.h +++ b/opencl/source/command_queue/enqueue_common.h @@ -486,8 +486,7 @@ BlitProperties CommandQueueHw::processDispatchForBlitEnqueue(const Mu blitProperties.outputTimestampPacket = currentTimestampPacketNode; if (isCacheFlushForBcsRequired()) { - auto cacheFlushTimestampPacketGpuAddress = timestampPacketDependencies.cacheFlushNodes.peekNodes()[0]->getGpuAddress() + - offsetof(TimestampPacketStorage, packets[0].contextEnd); + auto cacheFlushTimestampPacketGpuAddress = TimestampPacketHelper::getContextEndGpuAddress(*timestampPacketDependencies.cacheFlushNodes.peekNodes()[0]); PipeControlArgs args(true); MemorySynchronizationCommands::addPipeControlAndProgramPostSyncOperation( commandStream, @@ -553,7 +552,7 @@ void CommandQueueHw::processDispatchForCacheFlush(Surface **surfaces, uint64_t postSyncAddress = 0; if (getGpgpuCommandStreamReceiver().peekTimestampPacketWriteEnabled()) { auto timestampPacketNodeForPostSync = timestampPacketContainer->peekNodes().at(0); - postSyncAddress = timestampPacketNodeForPostSync->getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); + postSyncAddress = TimestampPacketHelper::getContextEndGpuAddress(*timestampPacketNodeForPostSync); } submitCacheFlush(surfaces, numSurfaces, commandStream, postSyncAddress); diff --git a/opencl/source/command_queue/gpgpu_walker_bdw_plus.inl b/opencl/source/command_queue/gpgpu_walker_bdw_plus.inl index 43599ed70e..c452fe3b20 100644 --- a/opencl/source/command_queue/gpgpu_walker_bdw_plus.inl +++ b/opencl/source/command_queue/gpgpu_walker_bdw_plus.inl @@ -187,7 +187,7 @@ void GpgpuWalkerHelper::setupTimestampPacket( const RootDeviceEnvironment &rootDeviceEnvironment) { if (TimestampPacketStorage::WriteOperationType::AfterWalker == writeOperationType) { - uint64_t address = timestampPacketNode->getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); + uint64_t address = TimestampPacketHelper::getContextEndGpuAddress(*timestampPacketNode); PipeControlArgs args; MemorySynchronizationCommands::addPipeControlAndProgramPostSyncOperation( *cmdStream, diff --git a/opencl/source/command_queue/hardware_interface_base.inl b/opencl/source/command_queue/hardware_interface_base.inl index d5028024d6..8201b052fd 100644 --- a/opencl/source/command_queue/hardware_interface_base.inl +++ b/opencl/source/command_queue/hardware_interface_base.inl @@ -97,7 +97,7 @@ void HardwareInterface::dispatchWalker( uint64_t postSyncAddress = 0; if (commandQueue.getGpgpuCommandStreamReceiver().peekTimestampPacketWriteEnabled()) { auto timestampPacketNodeForPostSync = currentTimestampPacketNodes->peekNodes().at(currentDispatchIndex); - postSyncAddress = timestampPacketNodeForPostSync->getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); + postSyncAddress = TimestampPacketHelper::getContextEndGpuAddress(*timestampPacketNodeForPostSync); } HardwareCommandsHelper::programCacheFlushAfterWalkerCommand(commandStream, commandQueue, mainKernel, postSyncAddress); } diff --git a/opencl/test/unit_test/command_queue/blit_enqueue_tests.cpp b/opencl/test/unit_test/command_queue/blit_enqueue_tests.cpp index 9201994cc2..4bdf6b21c5 100644 --- a/opencl/test/unit_test/command_queue/blit_enqueue_tests.cpp +++ b/opencl/test/unit_test/command_queue/blit_enqueue_tests.cpp @@ -438,7 +438,7 @@ HWTEST_TEMPLATED_F(BlitAuxTranslationTests, givenBlitTranslationWhenConstructing mockCmdQ->enqueueKernel(mockKernel->mockKernel, 1, nullptr, gws, nullptr, 0, nullptr, nullptr); auto kernelNode = mockCmdQ->timestampPacketContainer->peekNodes()[0]; - auto kernelNodeAddress = kernelNode->getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); + auto kernelNodeAddress = TimestampPacketHelper::getContextEndGpuAddress(*kernelNode); auto cmdList = getCmdList(bcsCsr->getCS(0)); @@ -513,7 +513,7 @@ HWTEST_TEMPLATED_F(BlitAuxTranslationTests, givenBlitTranslationWhenConstructing commandQueue->enqueueKernel(mockKernel->mockKernel, 1, nullptr, gws, nullptr, 1, clEvent, nullptr); - auto eventDependencyAddress = eventDependency->getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); + auto eventDependencyAddress = TimestampPacketHelper::getContextEndGpuAddress(*eventDependency); auto cmdList = getCmdList(bcsCsr->getCS(0)); @@ -550,11 +550,11 @@ HWTEST_TEMPLATED_F(BlitAuxTranslationTests, givenOutEventWhenDispatchingThenAssi // NonAux to Aux cmdFound = expectCommand(++cmdFound, cmdListQueue.end()); - auto eventNodeAddress = eventNodes[1]->getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); + auto eventNodeAddress = TimestampPacketHelper::getContextEndGpuAddress(*eventNodes[1]); verifySemaphore(cmdFound, eventNodeAddress); cmdFound = expectCommand(++cmdFound, cmdListQueue.end()); - eventNodeAddress = eventNodes[2]->getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); + eventNodeAddress = TimestampPacketHelper::getContextEndGpuAddress(*eventNodes[2]); verifySemaphore(cmdFound, eventNodeAddress); clReleaseEvent(clEvent); @@ -683,7 +683,7 @@ HWTEST_TEMPLATED_F(BlitAuxTranslationTests, givenBlitTranslationWhenConstructing commandQueue->enqueueKernel(mockKernel->mockKernel, 1, nullptr, gws, nullptr, 2, waitlist, nullptr); userEvent.setStatus(CL_COMPLETE); - auto eventDependencyAddress = eventDependency->getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); + auto eventDependencyAddress = TimestampPacketHelper::getContextEndGpuAddress(*eventDependency); auto cmdList = getCmdList(bcsCsr->getCS(0)); @@ -715,7 +715,7 @@ HWTEST_TEMPLATED_F(BlitAuxTranslationTests, givenBlitTranslationWhenConstructing userEvent.setStatus(CL_COMPLETE); auto kernelNode = mockCmdQ->timestampPacketContainer->peekNodes()[0]; - auto kernelNodeAddress = kernelNode->getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); + auto kernelNodeAddress = TimestampPacketHelper::getContextEndGpuAddress(*kernelNode); auto cmdList = getCmdList(bcsCsr->getCS(0)); diff --git a/opencl/test/unit_test/helpers/timestamp_packet_tests.cpp b/opencl/test/unit_test/helpers/timestamp_packet_tests.cpp index f6f0adecf7..dd11dfa9a9 100644 --- a/opencl/test/unit_test/helpers/timestamp_packet_tests.cpp +++ b/opencl/test/unit_test/helpers/timestamp_packet_tests.cpp @@ -80,7 +80,7 @@ struct TimestampPacketTests : public TimestampPacketSimpleTests { EXPECT_EQ(1u, semaphoreCmd->getSemaphoreDataDword()); uint64_t compareOffset = packetId * sizeof(TimestampPacketStorage::Packet); - auto dataAddress = timestampPacketNode->getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd) + compareOffset; + auto dataAddress = TimestampPacketHelper::getContextEndGpuAddress(*timestampPacketNode) + compareOffset; EXPECT_EQ(dataAddress, semaphoreCmd->getSemaphoreGraphicsAddress()); }; @@ -89,7 +89,7 @@ struct TimestampPacketTests : public TimestampPacketSimpleTests { void verifyMiAtomic(typename GfxFamily::MI_ATOMIC *miAtomicCmd, TagNode *timestampPacketNode) { using MI_ATOMIC = typename GfxFamily::MI_ATOMIC; EXPECT_NE(nullptr, miAtomicCmd); - auto writeAddress = timestampPacketNode->getGpuAddress() + offsetof(TimestampPacketStorage, implicitGpuDependenciesCount); + auto writeAddress = TimestampPacketHelper::getGpuDependenciesCountGpuAddress(*timestampPacketNode); EXPECT_EQ(MI_ATOMIC::ATOMIC_OPCODES::ATOMIC_4B_INCREMENT, miAtomicCmd->getAtomicOpcode()); EXPECT_EQ(writeAddress, UnitTestHelper::getMemoryAddress(*miAtomicCmd)); @@ -191,6 +191,19 @@ HWTEST_F(TimestampPacketTests, givenTagNodeWithPacketsUsed2WhenSemaphoreAndAtomi verifyMiAtomic(genCmdCast(*it++), &mockNode); } +TEST_F(TimestampPacketTests, givenTagNodeWhatAskingForGpuAddressesThenReturnCorrectValue) { + TimestampPacketStorage tag; + MockTagNode mockNode; + mockNode.tagForCpuAccess = &tag; + mockNode.gpuAddress = 0x1230000; + + auto expectedEndAddress = mockNode.getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); + EXPECT_EQ(expectedEndAddress, TimestampPacketHelper::getContextEndGpuAddress(mockNode)); + + auto expectedCounterAddress = mockNode.getGpuAddress() + offsetof(TimestampPacketStorage, implicitGpuDependenciesCount); + EXPECT_EQ(expectedCounterAddress, TimestampPacketHelper::getGpuDependenciesCountGpuAddress(mockNode)); +} + TEST_F(TimestampPacketSimpleTests, whenEndTagIsNotOneThenMarkAsCompleted) { TimestampPacketStorage timestampPacketStorage; auto &packet = timestampPacketStorage.packets[0]; @@ -596,7 +609,7 @@ HWCMDTEST_F(IGFX_GEN8_CORE, TimestampPacketTests, givenTimestampPacketWhenDispat } auto pipeControl = genCmdCast(*++it); EXPECT_NE(nullptr, pipeControl); - auto expectedAddress = timestampPacket.getNode(walkersFound)->getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); + auto expectedAddress = TimestampPacketHelper::getContextEndGpuAddress(*timestampPacket.getNode(walkersFound)); EXPECT_EQ(1u, pipeControl->getCommandStreamerStallEnable()); EXPECT_EQ(PIPE_CONTROL::POST_SYNC_OPERATION_WRITE_IMMEDIATE_DATA, pipeControl->getPostSyncOperation()); diff --git a/opencl/test/unit_test/mem_obj/buffer_tests.cpp b/opencl/test/unit_test/mem_obj/buffer_tests.cpp index 52403df576..378e538f8b 100644 --- a/opencl/test/unit_test/mem_obj/buffer_tests.cpp +++ b/opencl/test/unit_test/mem_obj/buffer_tests.cpp @@ -1105,13 +1105,13 @@ HWTEST_TEMPLATED_F(BcsBufferTests, givenWriteBufferEnqueueWhenProgrammingCommand continue; } semaphoresCount++; - auto dataAddress = timestampPacketNode->getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); + auto dataAddress = TimestampPacketHelper::getContextEndGpuAddress(*timestampPacketNode); EXPECT_EQ(dataAddress, semaphoreCmd->getSemaphoreGraphicsAddress()); EXPECT_EQ(0u, miAtomicsCount); } else if (auto miAtomicCmd = genCmdCast(cmd)) { miAtomicsCount++; - auto dataAddress = timestampPacketNode->getGpuAddress() + offsetof(TimestampPacketStorage, implicitGpuDependenciesCount); + auto dataAddress = TimestampPacketHelper::getGpuDependenciesCountGpuAddress(*timestampPacketNode); EXPECT_EQ(MI_ATOMIC::ATOMIC_OPCODES::ATOMIC_4B_INCREMENT, miAtomicCmd->getAtomicOpcode()); EXPECT_EQ(dataAddress, UnitTestHelper::getMemoryAddress(*miAtomicCmd)); EXPECT_EQ(1u, semaphoresCount); @@ -1150,13 +1150,13 @@ HWTEST_TEMPLATED_F(BcsBufferTests, givenReadBufferEnqueueWhenProgrammingCommandS continue; } semaphoresCount++; - auto dataAddress = timestampPacketNode->getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); + auto dataAddress = TimestampPacketHelper::getContextEndGpuAddress(*timestampPacketNode); EXPECT_EQ(dataAddress, semaphoreCmd->getSemaphoreGraphicsAddress()); EXPECT_EQ(0u, miAtomicsCount); } else if (auto miAtomicCmd = genCmdCast(cmd)) { miAtomicsCount++; - auto dataAddress = timestampPacketNode->getGpuAddress() + offsetof(TimestampPacketStorage, implicitGpuDependenciesCount); + auto dataAddress = TimestampPacketHelper::getGpuDependenciesCountGpuAddress(*timestampPacketNode); EXPECT_EQ(MI_ATOMIC::ATOMIC_OPCODES::ATOMIC_4B_INCREMENT, miAtomicCmd->getAtomicOpcode()); EXPECT_EQ(dataAddress, UnitTestHelper::getMemoryAddress(*miAtomicCmd)); EXPECT_EQ(1u, semaphoresCount); @@ -1404,8 +1404,8 @@ HWTEST_TEMPLATED_F(BcsBufferTests, givenOutputTimestampPacketWhenBlitCalledThenP void *hostPtr = reinterpret_cast(0x12340000); cmdQ->enqueueWriteBuffer(buffer.get(), true, 0, 1, hostPtr, nullptr, 0, nullptr, nullptr); - auto outputTimestampPacket = cmdQ->timestampPacketContainer->peekNodes().at(0); - auto timestampPacketGpuWriteAddress = outputTimestampPacket->getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); + auto outputTimestampPacket = cmdQ->timestampPacketContainer->peekNodes()[0]; + auto timestampPacketGpuWriteAddress = TimestampPacketHelper::getContextEndGpuAddress(*outputTimestampPacket); HardwareParse hwParser; hwParser.parseCommands(csr->commandStream); 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 e74f00cb08..e5e0957e92 100644 --- a/shared/source/command_stream/command_stream_receiver_hw_base.inl +++ b/shared/source/command_stream/command_stream_receiver_hw_base.inl @@ -537,8 +537,7 @@ inline void CommandStreamReceiverHw::programStallingPipeControlForBar auto barrierTimestampPacketNodes = dispatchFlags.barrierTimestampPacketNodes; if (barrierTimestampPacketNodes && barrierTimestampPacketNodes->peekNodes().size() != 0) { - auto barrierTimestampPacketGpuAddress = dispatchFlags.barrierTimestampPacketNodes->peekNodes()[0]->getGpuAddress() + - offsetof(TimestampPacketStorage, packets[0].contextEnd); + auto barrierTimestampPacketGpuAddress = TimestampPacketHelper::getContextEndGpuAddress(*dispatchFlags.barrierTimestampPacketNodes->peekNodes()[0]); PipeControlArgs args(true); MemorySynchronizationCommands::addPipeControlAndProgramPostSyncOperation( @@ -863,7 +862,7 @@ uint32_t CommandStreamReceiverHw::blitBuffer(const BlitPropertiesCont BlitCommandsHelper::dispatchBlitCommandsForBuffer(blitProperties, commandStream, *this->executionEnvironment.rootDeviceEnvironments[this->rootDeviceIndex]); if (blitProperties.outputTimestampPacket) { - auto timestampPacketGpuAddress = blitProperties.outputTimestampPacket->getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); + auto timestampPacketGpuAddress = TimestampPacketHelper::getContextEndGpuAddress(*blitProperties.outputTimestampPacket); EncodeMiFlushDW::programMiFlushDw(commandStream, timestampPacketGpuAddress, 0, true, true); makeResident(*blitProperties.outputTimestampPacket->getBaseGraphicsAllocation()); } diff --git a/shared/source/helpers/timestamp_packet.h b/shared/source/helpers/timestamp_packet.h index 9f6b987f79..65db946648 100644 --- a/shared/source/helpers/timestamp_packet.h +++ b/shared/source/helpers/timestamp_packet.h @@ -110,14 +110,22 @@ struct TimestampPacketDependencies : public NonCopyableClass { }; struct TimestampPacketHelper { + static uint64_t getContextEndGpuAddress(const TagNode ×tampPacketNode) { + return timestampPacketNode.getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); + } + + static uint64_t getGpuDependenciesCountGpuAddress(const TagNode ×tampPacketNode) { + return timestampPacketNode.getGpuAddress() + offsetof(TimestampPacketStorage, implicitGpuDependenciesCount); + } + template static void programSemaphoreWithImplicitDependency(LinearStream &cmdStream, TagNode ×tampPacketNode, uint32_t numSupportedDevices) { using MI_ATOMIC = typename GfxFamily::MI_ATOMIC; using COMPARE_OPERATION = typename GfxFamily::MI_SEMAPHORE_WAIT::COMPARE_OPERATION; using MI_SEMAPHORE_WAIT = typename GfxFamily::MI_SEMAPHORE_WAIT; - auto compareAddress = timestampPacketNode.getGpuAddress() + offsetof(TimestampPacketStorage, packets[0].contextEnd); - auto dependenciesCountAddress = timestampPacketNode.getGpuAddress() + offsetof(TimestampPacketStorage, implicitGpuDependenciesCount); + auto compareAddress = getContextEndGpuAddress(timestampPacketNode); + auto dependenciesCountAddress = getGpuDependenciesCountGpuAddress(timestampPacketNode); for (uint32_t packetId = 0; packetId < timestampPacketNode.tagForCpuAccess->packetsUsed; packetId++) { uint64_t compareOffset = packetId * sizeof(TimestampPacketStorage::Packet); @@ -161,8 +169,7 @@ struct TimestampPacketHelper { // cache flush after NDR, before NonAuxToAux if (auxTranslationDirection == AuxTranslationDirection::NonAuxToAux && timestampPacketDependencies->cacheFlushNodes.peekNodes().size() > 0) { UNRECOVERABLE_IF(timestampPacketDependencies->cacheFlushNodes.peekNodes().size() != 1); - auto cacheFlushTimestampPacketGpuAddress = timestampPacketDependencies->cacheFlushNodes.peekNodes()[0]->getGpuAddress() + - offsetof(TimestampPacketStorage, packets[0].contextEnd); + auto cacheFlushTimestampPacketGpuAddress = getContextEndGpuAddress(*timestampPacketDependencies->cacheFlushNodes.peekNodes()[0]); PipeControlArgs args(true); MemorySynchronizationCommands::addPipeControlAndProgramPostSyncOperation(