From f3d17008ee901811b41f44667b7c04abdb759b92 Mon Sep 17 00:00:00 2001 From: Mateusz Jablonski Date: Mon, 1 Apr 2019 12:42:26 +0200 Subject: [PATCH] TransferProperties: lock resource only when transfer on CPU is requested Change-Id: Ic93b4fd438e75f5d54cbae9bec332c4b18c6b1ee Signed-off-by: Mateusz Jablonski --- runtime/command_queue/command_queue.cpp | 6 ++-- runtime/command_queue/enqueue_read_buffer.h | 4 +-- runtime/command_queue/enqueue_write_buffer.h | 4 +-- runtime/helpers/properties_helper.cpp | 6 ++-- runtime/helpers/properties_helper.h | 2 +- .../command_queue/command_queue_hw_tests.cpp | 2 +- .../helpers/transfer_properties_tests.cpp | 32 ++++++++++++++----- 7 files changed, 37 insertions(+), 19 deletions(-) diff --git a/runtime/command_queue/command_queue.cpp b/runtime/command_queue/command_queue.cpp index ff4ab6333d..d41d8d87cd 100644 --- a/runtime/command_queue/command_queue.cpp +++ b/runtime/command_queue/command_queue.cpp @@ -419,7 +419,7 @@ void *CommandQueue::enqueueMapBuffer(Buffer *buffer, cl_bool blockingMap, const cl_event *eventWaitList, cl_event *event, cl_int &errcodeRet) { - TransferProperties transferProperties(buffer, CL_COMMAND_MAP_BUFFER, mapFlags, blockingMap != CL_FALSE, &offset, &size, nullptr); + TransferProperties transferProperties(buffer, CL_COMMAND_MAP_BUFFER, mapFlags, blockingMap != CL_FALSE, &offset, &size, nullptr, false); EventsRequest eventsRequest(numEventsInWaitList, eventWaitList, event); return enqueueMapMemObject(transferProperties, eventsRequest, errcodeRet); @@ -433,7 +433,7 @@ void *CommandQueue::enqueueMapImage(Image *image, cl_bool blockingMap, const cl_event *eventWaitList, cl_event *event, cl_int &errcodeRet) { TransferProperties transferProperties(image, CL_COMMAND_MAP_IMAGE, mapFlags, blockingMap != CL_FALSE, - const_cast(origin), const_cast(region), nullptr); + const_cast(origin), const_cast(region), nullptr, false); EventsRequest eventsRequest(numEventsInWaitList, eventWaitList, event); if (image->isMemObjZeroCopy() && image->mappingOnCpuAllowed()) { @@ -459,7 +459,7 @@ void *CommandQueue::enqueueMapImage(Image *image, cl_bool blockingMap, cl_int CommandQueue::enqueueUnmapMemObject(MemObj *memObj, void *mappedPtr, cl_uint numEventsInWaitList, const cl_event *eventWaitList, cl_event *event) { - TransferProperties transferProperties(memObj, CL_COMMAND_UNMAP_MEM_OBJECT, 0, false, nullptr, nullptr, mappedPtr); + TransferProperties transferProperties(memObj, CL_COMMAND_UNMAP_MEM_OBJECT, 0, false, nullptr, nullptr, mappedPtr, false); EventsRequest eventsRequest(numEventsInWaitList, eventWaitList, event); return enqueueUnmapMemObject(transferProperties, eventsRequest); diff --git a/runtime/command_queue/enqueue_read_buffer.h b/runtime/command_queue/enqueue_read_buffer.h index 2208d9cc1e..0e8f1ffc77 100644 --- a/runtime/command_queue/enqueue_read_buffer.h +++ b/runtime/command_queue/enqueue_read_buffer.h @@ -40,7 +40,7 @@ cl_int CommandQueueHw::enqueueReadBuffer( buffer->isReadWriteOnCpuAllowed(blockingRead, numEventsInWaitList, ptr, size)) && context->getDevice(0)->getDeviceInfo().cpuCopyAllowed) { if (!isMemTransferNeeded) { - TransferProperties transferProperties(buffer, CL_COMMAND_MARKER, 0, true, &offset, &size, ptr); + TransferProperties transferProperties(buffer, CL_COMMAND_MARKER, 0, true, &offset, &size, ptr, true); EventsRequest eventsRequest(numEventsInWaitList, eventWaitList, event); cpuDataTransferHandler(transferProperties, eventsRequest, retVal); if (event) { @@ -53,7 +53,7 @@ cl_int CommandQueueHw::enqueueReadBuffer( } return retVal; } - TransferProperties transferProperties(buffer, CL_COMMAND_READ_BUFFER, 0, true, &offset, &size, ptr); + TransferProperties transferProperties(buffer, CL_COMMAND_READ_BUFFER, 0, true, &offset, &size, ptr, false); EventsRequest eventsRequest(numEventsInWaitList, eventWaitList, event); cpuDataTransferHandler(transferProperties, eventsRequest, retVal); return retVal; diff --git a/runtime/command_queue/enqueue_write_buffer.h b/runtime/command_queue/enqueue_write_buffer.h index d5a0363a08..f7c9b18a5b 100644 --- a/runtime/command_queue/enqueue_write_buffer.h +++ b/runtime/command_queue/enqueue_write_buffer.h @@ -37,7 +37,7 @@ cl_int CommandQueueHw::enqueueWriteBuffer( buffer->isReadWriteOnCpuAllowed(blockingWrite, numEventsInWaitList, const_cast(ptr), size)) && context->getDevice(0)->getDeviceInfo().cpuCopyAllowed) { if (!isMemTransferNeeded) { - TransferProperties transferProperties(buffer, CL_COMMAND_MARKER, 0, true, &offset, &size, const_cast(ptr)); + TransferProperties transferProperties(buffer, CL_COMMAND_MARKER, 0, true, &offset, &size, const_cast(ptr), true); EventsRequest eventsRequest(numEventsInWaitList, eventWaitList, event); cpuDataTransferHandler(transferProperties, eventsRequest, retVal); @@ -51,7 +51,7 @@ cl_int CommandQueueHw::enqueueWriteBuffer( } return retVal; } - TransferProperties transferProperties(buffer, CL_COMMAND_WRITE_BUFFER, 0, true, &offset, &size, const_cast(ptr)); + TransferProperties transferProperties(buffer, CL_COMMAND_WRITE_BUFFER, 0, true, &offset, &size, const_cast(ptr), false); EventsRequest eventsRequest(numEventsInWaitList, eventWaitList, event); cpuDataTransferHandler(transferProperties, eventsRequest, retVal); return retVal; diff --git a/runtime/helpers/properties_helper.cpp b/runtime/helpers/properties_helper.cpp index 35a2fbcd9a..ccce6fb7f2 100644 --- a/runtime/helpers/properties_helper.cpp +++ b/runtime/helpers/properties_helper.cpp @@ -14,7 +14,7 @@ namespace NEO { TransferProperties::TransferProperties(MemObj *memObj, cl_command_type cmdType, cl_map_flags mapFlags, bool blocking, - size_t *offsetPtr, size_t *sizePtr, void *ptr) + size_t *offsetPtr, size_t *sizePtr, void *ptr, bool doTransferOnCpu) : memObj(memObj), cmdType(cmdType), mapFlags(mapFlags), blocking(blocking), ptr(ptr) { // no size or offset passed for unmap operation @@ -22,7 +22,9 @@ TransferProperties::TransferProperties(MemObj *memObj, cl_command_type cmdType, if (memObj->peekClMemObjType() == CL_MEM_OBJECT_BUFFER) { size[0] = *sizePtr; offset[0] = *offsetPtr; - if ((false == MemoryPool::isSystemMemoryPool(memObj->getGraphicsAllocation()->getMemoryPool())) && (memObj->getMemoryManager() != nullptr)) { + if (doTransferOnCpu && + (false == MemoryPool::isSystemMemoryPool(memObj->getGraphicsAllocation()->getMemoryPool())) && + (memObj->getMemoryManager() != nullptr)) { this->lockedPtr = memObj->getMemoryManager()->lockResource(memObj->getGraphicsAllocation()); } } else { diff --git a/runtime/helpers/properties_helper.h b/runtime/helpers/properties_helper.h index 1aa4e26e45..79cde2dc63 100644 --- a/runtime/helpers/properties_helper.h +++ b/runtime/helpers/properties_helper.h @@ -47,7 +47,7 @@ struct TransferProperties { TransferProperties() = delete; TransferProperties(MemObj *memObj, cl_command_type cmdType, cl_map_flags mapFlags, bool blocking, size_t *offsetPtr, size_t *sizePtr, - void *ptr); + void *ptr, bool doTransferOnCpu); MemObj *memObj = nullptr; cl_command_type cmdType = 0; diff --git a/unit_tests/command_queue/command_queue_hw_tests.cpp b/unit_tests/command_queue/command_queue_hw_tests.cpp index 068aaee79e..3fd045be26 100644 --- a/unit_tests/command_queue/command_queue_hw_tests.cpp +++ b/unit_tests/command_queue/command_queue_hw_tests.cpp @@ -723,7 +723,7 @@ HWTEST_F(CommandQueueHwTest, givenCommandQueueThatIsBlockedAndUsesCpuCopyWhenEve cmdQHw->taskLevel = Event::eventNotReady; size_t offset = 0; size_t size = 4096u; - TransferProperties transferProperties(&buffer, CL_COMMAND_READ_BUFFER, 0, false, &offset, &size, nullptr); + TransferProperties transferProperties(&buffer, CL_COMMAND_READ_BUFFER, 0, false, &offset, &size, nullptr, false); EventsRequest eventsRequest(0, nullptr, &returnEvent); cmdQHw->cpuDataTransferHandler(transferProperties, eventsRequest, retVal); EXPECT_EQ(CL_SUCCESS, retVal); diff --git a/unit_tests/helpers/transfer_properties_tests.cpp b/unit_tests/helpers/transfer_properties_tests.cpp index 0205c54279..eebbba4898 100644 --- a/unit_tests/helpers/transfer_properties_tests.cpp +++ b/unit_tests/helpers/transfer_properties_tests.cpp @@ -20,11 +20,11 @@ TEST(TransferPropertiesTest, givenTransferPropertiesCreatedWhenDefaultDebugSetti size_t offset = 0; size_t size = 4096u; - TransferProperties transferProperties(&buffer, CL_COMMAND_MAP_BUFFER, 0, false, &offset, &size, nullptr); + TransferProperties transferProperties(&buffer, CL_COMMAND_MAP_BUFFER, 0, false, &offset, &size, nullptr, true); EXPECT_EQ(nullptr, transferProperties.lockedPtr); } -TEST(TransferPropertiesTest, givenAllocationInNonSystemPoolWhenTransferPropertiesAreCreatedForMapBufferThenLockPtrIsSet) { +TEST(TransferPropertiesTest, givenAllocationInNonSystemPoolWhenTransferPropertiesAreCreatedForMapBufferAndCpuTransferIsRequestedThenLockPtrIsSet) { MockExecutionEnvironment executionEnvironment(*platformDevices); MemoryManagerCreate memoryManager(false, true, executionEnvironment); @@ -37,9 +37,25 @@ TEST(TransferPropertiesTest, givenAllocationInNonSystemPoolWhenTransferPropertie size_t offset = 0; size_t size = 4096u; - TransferProperties transferProperties(buffer.get(), CL_COMMAND_MAP_BUFFER, 0, false, &offset, &size, nullptr); + TransferProperties transferProperties(buffer.get(), CL_COMMAND_MAP_BUFFER, 0, false, &offset, &size, nullptr, true); EXPECT_NE(nullptr, transferProperties.lockedPtr); } +TEST(TransferPropertiesTest, givenAllocationInNonSystemPoolWhenTransferPropertiesAreCreatedForMapBufferAndCpuTransferIsNotRequestedThenLockPtrIsNotSet) { + MockExecutionEnvironment executionEnvironment(*platformDevices); + MemoryManagerCreate memoryManager(false, true, executionEnvironment); + + MockContext ctx; + ctx.setMemoryManager(&memoryManager); + cl_int retVal; + std::unique_ptr buffer(Buffer::create(&ctx, 0, 1, nullptr, retVal)); + static_cast(buffer->getGraphicsAllocation())->overrideMemoryPool(MemoryPool::SystemCpuInaccessible); + + size_t offset = 0; + size_t size = 4096u; + + TransferProperties transferProperties(buffer.get(), CL_COMMAND_MAP_BUFFER, 0, false, &offset, &size, nullptr, false); + EXPECT_EQ(nullptr, transferProperties.lockedPtr); +} TEST(TransferPropertiesTest, givenAllocationInSystemPoolWhenTransferPropertiesAreCreatedForMapBufferThenLockPtrIsNotSet) { MockExecutionEnvironment executionEnvironment(*platformDevices); @@ -54,7 +70,7 @@ TEST(TransferPropertiesTest, givenAllocationInSystemPoolWhenTransferPropertiesAr size_t offset = 0; size_t size = 4096u; - TransferProperties transferProperties(buffer.get(), CL_COMMAND_MAP_BUFFER, 0, false, &offset, &size, nullptr); + TransferProperties transferProperties(buffer.get(), CL_COMMAND_MAP_BUFFER, 0, false, &offset, &size, nullptr, true); EXPECT_EQ(nullptr, transferProperties.lockedPtr); } @@ -63,7 +79,7 @@ TEST(TransferPropertiesTest, givenTransferPropertiesCreatedWhenMemoryManagerInMe size_t offset = 0; size_t size = 4096u; - TransferProperties transferProperties(&buffer, CL_COMMAND_MAP_BUFFER, 0, false, &offset, &size, nullptr); + TransferProperties transferProperties(&buffer, CL_COMMAND_MAP_BUFFER, 0, false, &offset, &size, nullptr, true); EXPECT_EQ(nullptr, transferProperties.lockedPtr); } @@ -80,7 +96,7 @@ TEST(TransferPropertiesTest, givenTransferPropertiesWhenLockedPtrIsSetThenItIsRe size_t offset = 0; size_t size = 4096u; - TransferProperties transferProperties(buffer.get(), CL_COMMAND_MAP_BUFFER, 0, false, &offset, &size, nullptr); + TransferProperties transferProperties(buffer.get(), CL_COMMAND_MAP_BUFFER, 0, false, &offset, &size, nullptr, true); ASSERT_NE(nullptr, transferProperties.lockedPtr); EXPECT_EQ(transferProperties.lockedPtr, transferProperties.getCpuPtrForReadWrite()); } @@ -90,7 +106,7 @@ TEST(TransferPropertiesTest, givenTransferPropertiesWhenLockedPtrIsNotSetThenItI size_t offset = 0; size_t size = 4096u; - TransferProperties transferProperties(&buffer, CL_COMMAND_MAP_BUFFER, 0, false, &offset, &size, nullptr); + TransferProperties transferProperties(&buffer, CL_COMMAND_MAP_BUFFER, 0, false, &offset, &size, nullptr, true); ASSERT_EQ(nullptr, transferProperties.lockedPtr); EXPECT_NE(transferProperties.lockedPtr, transferProperties.getCpuPtrForReadWrite()); } @@ -104,7 +120,7 @@ TEST(TransferPropertiesTest, givenTransferPropertiesWhenLockedPtrIsSetThenLocked size_t offset = 0; size_t size = 4096u; - TransferProperties transferProperties(&buffer, CL_COMMAND_MAP_BUFFER, 0, false, &offset, &size, nullptr); + TransferProperties transferProperties(&buffer, CL_COMMAND_MAP_BUFFER, 0, false, &offset, &size, nullptr, true); transferProperties.lockedPtr = lockedPtr; auto expectedPtr = ptrOffset(lockedPtr, memObjOffset); EXPECT_EQ(expectedPtr, transferProperties.getCpuPtrForReadWrite());