From b99cf6c3ff8a70b9e2fa214c2fb0b56f0ec6d47d Mon Sep 17 00:00:00 2001 From: "Mrozek, Michal" Date: Wed, 19 Dec 2018 20:15:32 +0100 Subject: [PATCH] Image / Buffer refactor. - do not use redescribe flag for image/buffer from other image/buffer. - use redescribe flag only when image is redescribed via redescribe interface - remove image specific functions from mem object - remove redundant fields - add new implementation of isImageFromBuffer/isImageFromImage basing on associated mem object. - pass associated mem object to redescrbed images. - remove redundant setters Change-Id: I267637a48fbc2afdad9a9f5e5e9ccd6bd0c09972 --- runtime/kernel/kernel.cpp | 4 +-- runtime/mem_obj/buffer.cpp | 2 +- runtime/mem_obj/image.cpp | 15 +++-------- runtime/mem_obj/image.h | 3 +++ runtime/mem_obj/mem_obj.cpp | 4 +-- runtime/mem_obj/mem_obj.h | 6 ----- .../command_queue/enqueue_map_image_tests.cpp | 1 + unit_tests/mem_obj/image1d_tests.cpp | 26 ++++++------------- .../mem_obj/image2d_from_buffer_tests.cpp | 9 +++++++ unit_tests/mem_obj/image_tests.cpp | 8 +----- 10 files changed, 31 insertions(+), 47 deletions(-) diff --git a/runtime/kernel/kernel.cpp b/runtime/kernel/kernel.cpp index 079b6ef547..6d71047b92 100644 --- a/runtime/kernel/kernel.cpp +++ b/runtime/kernel/kernel.cpp @@ -935,8 +935,8 @@ inline void Kernel::makeArgsResident(CommandStreamReceiver &commandStreamReceive } else if (Kernel::isMemObj(kernelArguments[argIndex].type)) { auto clMem = const_cast(static_cast(kernelArguments[argIndex].object)); auto memObj = castToObjectOrAbort(clMem); - DEBUG_BREAK_IF(memObj == nullptr); - if (memObj->isImageFromImage()) { + auto image = castToObject(clMem); + if (image && image->isImageFromImage()) { commandStreamReceiver.setSamplerCacheFlushRequired(CommandStreamReceiver::SamplerCacheFlushState::samplerCacheFlushBefore); } commandStreamReceiver.makeResident(*memObj->getGraphicsAllocation()); diff --git a/runtime/mem_obj/buffer.cpp b/runtime/mem_obj/buffer.cpp index a8973e9857..fa24558927 100644 --- a/runtime/mem_obj/buffer.cpp +++ b/runtime/mem_obj/buffer.cpp @@ -352,7 +352,7 @@ Buffer *Buffer::createSubBuffer(cl_mem_flags flags, ptrOffset(this->memoryStorage, region->origin), this->hostPtr ? ptrOffset(this->hostPtr, region->origin) : nullptr, this->graphicsAllocation, - this->isZeroCopy, this->isHostPtrSVM, true); + this->isZeroCopy, this->isHostPtrSVM, false); if (this->context->isProvidingPerformanceHints()) { this->context->providePerformanceHint(CL_CONTEXT_DIAGNOSTICS_LEVEL_GOOD_INTEL, SUBBUFFER_SHARES_MEMORY, static_cast(this)); diff --git a/runtime/mem_obj/image.cpp b/runtime/mem_obj/image.cpp index 3770343e24..9b50bb370c 100644 --- a/runtime/mem_obj/image.cpp +++ b/runtime/mem_obj/image.cpp @@ -114,7 +114,6 @@ Image *Image::create(Context *context, MemoryManager *memoryManager = context->getMemoryManager(); Buffer *parentBuffer = castToObject(imageDesc->mem_object); Image *parentImage = castToObject(imageDesc->mem_object); - bool isImageFromBuffer = false; do { size_t imageWidth = imageDesc->image_width; @@ -181,10 +180,7 @@ Image *Image::create(Context *context, bool zeroCopy = false; bool transferNeeded = false; - bool imageRedescribed = false; if (((imageDesc->image_type == CL_MEM_OBJECT_IMAGE1D_BUFFER) || (imageDesc->image_type == CL_MEM_OBJECT_IMAGE2D)) && (parentBuffer != nullptr)) { - isImageFromBuffer = true; - imageRedescribed = true; memory = parentBuffer->getGraphicsAllocation(); // Image from buffer - we never allocate memory, we use what buffer provides zeroCopy = true; @@ -289,7 +285,7 @@ Image *Image::create(Context *context, } image = createImageHw(context, flags, imgInfo.size, hostPtrToSet, surfaceFormat->OCLImageFormat, - imageDescriptor, zeroCopy, memory, imageRedescribed, isTilingAllowed, 0, 0, surfaceFormat); + imageDescriptor, zeroCopy, memory, false, isTilingAllowed, 0, 0, surfaceFormat); if (imageDesc->image_type != CL_MEM_OBJECT_IMAGE1D_ARRAY && imageDesc->image_type != CL_MEM_OBJECT_IMAGE2D_ARRAY) { image->imageDesc.image_array_size = 0; @@ -297,9 +293,7 @@ Image *Image::create(Context *context, if ((imageDesc->image_type == CL_MEM_OBJECT_IMAGE1D_BUFFER) || ((imageDesc->image_type == CL_MEM_OBJECT_IMAGE2D) && (imageDesc->mem_object != nullptr))) { image->associatedMemObject = castToObject(imageDesc->mem_object); } - if (parentImage) { - image->isImageFromImageCreated = true; - } + // Driver needs to store rowPitch passed by the app in order to synchronize the host_ptr later on map call image->setHostPtrRowPitch(imageDesc->image_row_pitch ? imageDesc->image_row_pitch : hostPtrRowPitch); image->setHostPtrSlicePitch(hostPtrSlicePitch); @@ -310,7 +304,6 @@ Image *Image::create(Context *context, image->setQPitch(imgInfo.qPitch); image->setSurfaceOffsets(imgInfo.offset, imgInfo.xOffset, imgInfo.yOffset, imgInfo.yOffsetForUVPlane); image->setMipCount(imgInfo.mipCount); - image->setIsImageFromBuffer(isImageFromBuffer); if (parentImage) { image->setMediaPlaneType(static_cast(imageDesc->image_depth)); image->setParentSharingHandler(parentImage->getSharingHandler()); @@ -838,7 +831,7 @@ Image *Image::redescribeFillImage() { &this->surfaceOffsets); image->setQPitch(this->getQPitch()); image->setCubeFaceIndex(this->getCubeFaceIndex()); - image->setIsImageFromBuffer(this->isImageFromBuffer()); + image->associatedMemObject = this->associatedMemObject; return image; } @@ -886,7 +879,7 @@ Image *Image::redescribe() { &this->surfaceOffsets); image->setQPitch(this->getQPitch()); image->setCubeFaceIndex(this->getCubeFaceIndex()); - image->setIsImageFromBuffer(this->isImageFromBuffer()); + image->associatedMemObject = this->associatedMemObject; return image; } diff --git a/runtime/mem_obj/image.h b/runtime/mem_obj/image.h index 503460a9b7..1efa875806 100644 --- a/runtime/mem_obj/image.h +++ b/runtime/mem_obj/image.h @@ -177,6 +177,9 @@ class Image : public MemObj { bool hasSameDescriptor(const cl_image_desc &imageDesc) const; bool hasValidParentImageFormat(const cl_image_format &imageFormat) const; + bool isImageFromBuffer() const { return castToObject(static_cast(associatedMemObject)) ? true : false; } + bool isImageFromImage() const { return castToObject(static_cast(associatedMemObject)) ? true : false; } + protected: Image(Context *context, cl_mem_flags flags, diff --git a/runtime/mem_obj/mem_obj.cpp b/runtime/mem_obj/mem_obj.cpp index c391d75a8b..4e72ab9554 100644 --- a/runtime/mem_obj/mem_obj.cpp +++ b/runtime/mem_obj/mem_obj.cpp @@ -55,11 +55,11 @@ MemObj::~MemObj() { needWait = true; } - if (memoryManager) { + if (memoryManager && !isObjectRedescribed) { if (peekSharingHandler()) { peekSharingHandler()->releaseReusedGraphicsAllocation(); } - if (graphicsAllocation && !associatedMemObject && !isObjectRedescribed && !isHostPtrSVM && graphicsAllocation->peekReuseCount() == 0) { + if (graphicsAllocation && !associatedMemObject && !isHostPtrSVM && graphicsAllocation->peekReuseCount() == 0) { memoryManager->removeAllocationFromHostPtrManager(graphicsAllocation); bool doAsyncDestrucions = DebugManager.flags.EnableAsyncDestroyAllocations.get(); if (!doAsyncDestrucions) { diff --git a/runtime/mem_obj/mem_obj.h b/runtime/mem_obj/mem_obj.h index 8fb2413e46..cdc7609c02 100644 --- a/runtime/mem_obj/mem_obj.h +++ b/runtime/mem_obj/mem_obj.h @@ -85,10 +85,6 @@ class MemObj : public BaseObject<_cl_mem> { virtual bool allowTiling() const { return false; } - bool isImageFromImage() const { return isImageFromImageCreated; } - void setIsImageFromBuffer(bool isImageFromBuffer) { isImageFromBufferCreated = isImageFromBuffer; } - bool isImageFromBuffer() const { return isImageFromBufferCreated; } - void *getCpuAddressForMapping(); void *getCpuAddressForMemoryTransfer(); @@ -130,8 +126,6 @@ class MemObj : public BaseObject<_cl_mem> { bool isZeroCopy; bool isHostPtrSVM; bool isObjectRedescribed; - bool isImageFromImageCreated = false; - bool isImageFromBufferCreated = false; MemoryManager *memoryManager = nullptr; GraphicsAllocation *graphicsAllocation; GraphicsAllocation *mcsAllocation = nullptr; diff --git a/unit_tests/command_queue/enqueue_map_image_tests.cpp b/unit_tests/command_queue/enqueue_map_image_tests.cpp index 958459a2cd..7705830f78 100644 --- a/unit_tests/command_queue/enqueue_map_image_tests.cpp +++ b/unit_tests/command_queue/enqueue_map_image_tests.cpp @@ -209,6 +209,7 @@ HWTEST_F(EnqueueMapImageTest, givenTiledImageWhenMapImageIsCalledThenStorageIsSe region, nullptr, nullptr, 0, nullptr, nullptr, retVal); EXPECT_TRUE(mockImage.ownershipTaken); + mockImage.releaseAllocatedMapPtr(); } TEST_F(EnqueueMapImageTest, checkPointer) { diff --git a/unit_tests/mem_obj/image1d_tests.cpp b/unit_tests/mem_obj/image1d_tests.cpp index 4f9f51981a..bf07e03f6c 100644 --- a/unit_tests/mem_obj/image1d_tests.cpp +++ b/unit_tests/mem_obj/image1d_tests.cpp @@ -1,23 +1,8 @@ /* - * Copyright (c) 2017, Intel Corporation + * Copyright (C) 2017-2018 Intel Corporation * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: + * SPDX-License-Identifier: MIT * - * The above copyright notice and this permission notice shall be included - * in all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS - * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR - * OTHER DEALINGS IN THE SOFTWARE. */ #include "hw_cmds.h" @@ -122,6 +107,8 @@ HWTEST_P(CreateImage1DType, validTypes) { EXPECT_EQ(image->getCubeFaceIndex(), static_cast(__GMM_NO_CUBE_MAP)); ASSERT_EQ(true, image->isMemObjZeroCopy()); + EXPECT_FALSE(image->isImageFromImage()); + auto address = image->getCpuAddress(); EXPECT_NE(nullptr, address); @@ -129,8 +116,11 @@ HWTEST_P(CreateImage1DType, validTypes) { Buffer *inputBuffer = castToObject(imageDesc.buffer); EXPECT_NE(nullptr, inputBuffer->getCpuAddress()); EXPECT_EQ(inputBuffer->getCpuAddress(), image->getCpuAddress()); - EXPECT_TRUE(image->getIsObjectRedescribed()); + EXPECT_FALSE(image->getIsObjectRedescribed()); EXPECT_GE(2, inputBuffer->getRefInternalCount()); + EXPECT_TRUE(image->isImageFromBuffer()); + } else { + EXPECT_FALSE(image->isImageFromBuffer()); } typedef typename FamilyType::RENDER_SURFACE_STATE SURFACE_STATE; diff --git a/unit_tests/mem_obj/image2d_from_buffer_tests.cpp b/unit_tests/mem_obj/image2d_from_buffer_tests.cpp index 441713d9bf..0c05059102 100644 --- a/unit_tests/mem_obj/image2d_from_buffer_tests.cpp +++ b/unit_tests/mem_obj/image2d_from_buffer_tests.cpp @@ -241,6 +241,15 @@ TEST_F(Image2dFromBufferTest, InterceptBuffersHostPtr) { delete imageFromBuffer; } +TEST_F(Image2dFromBufferTest, givenImageFromBufferWhenItIsRedescribedThenItReturnsProperImageFromBufferValue) { + std::unique_ptr imageFromBuffer(createImage()); + EXPECT_TRUE(imageFromBuffer->isImageFromBuffer()); + std::unique_ptr redescribedImage(imageFromBuffer->redescribe()); + EXPECT_TRUE(redescribedImage->isImageFromBuffer()); + std::unique_ptr redescribedfillImage(imageFromBuffer->redescribeFillImage()); + EXPECT_TRUE(redescribedfillImage->isImageFromBuffer()); +} + TEST_F(Image2dFromBufferTest, givenMemoryManagerNotSupportingVirtualPaddingWhenImageIsCreatedThenPaddingIsNotApplied) { auto memoryManager = context.getMemoryManager(); memoryManager->setVirtualPaddingSupport(false); diff --git a/unit_tests/mem_obj/image_tests.cpp b/unit_tests/mem_obj/image_tests.cpp index b14789ffba..87295aec33 100644 --- a/unit_tests/mem_obj/image_tests.cpp +++ b/unit_tests/mem_obj/image_tests.cpp @@ -573,17 +573,11 @@ TEST_P(CreateImageHostPtr, getAddress) { } } -TEST_P(CreateImageHostPtr, getSize) { +TEST_P(CreateImageHostPtr, givenImageWhenItIsCreateItHasProperSizeAndGraphicsAllocation) { image = createImage(retVal); ASSERT_NE(nullptr, image); EXPECT_NE(0u, image->getSize()); -} - -TEST_P(CreateImageHostPtr, graphicsAllocationPresent) { - image = createImage(retVal); - ASSERT_NE(nullptr, image); - EXPECT_NE(nullptr, image->getGraphicsAllocation()); }