From 7a09c51f3de83085dff3794a22f1ceb5619cbba8 Mon Sep 17 00:00:00 2001 From: Dominik Dabek Date: Fri, 19 Jan 2024 15:09:27 +0000 Subject: [PATCH] fix(ocl): querying memobj offset Querying buffer offset via clGetMemObjectInfo should not modify its value. Related-To: NEO-9690 Signed-off-by: Dominik Dabek --- opencl/source/mem_obj/mem_obj.cpp | 12 +++--- .../mem_obj/buffer_pool_alloc_tests.cpp | 42 ++++++++++++++----- .../get_mem_object_info_subbuffer_tests.cpp | 6 ++- 3 files changed, 43 insertions(+), 17 deletions(-) diff --git a/opencl/source/mem_obj/mem_obj.cpp b/opencl/source/mem_obj/mem_obj.cpp index f0ea04ae40f1f..4d9105f6e79d8 100644 --- a/opencl/source/mem_obj/mem_obj.cpp +++ b/opencl/source/mem_obj/mem_obj.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2018-2023 Intel Corporation + * Copyright (C) 2018-2024 Intel Corporation * * SPDX-License-Identifier: MIT * @@ -127,6 +127,7 @@ cl_int MemObj::getMemObjectInfo(cl_mem_info paramName, size_t srcParamSize = GetInfo::invalidSourceSize; void *srcParam = nullptr; cl_bool usesSVMPointer; + size_t clOffset = 0; cl_uint refCnt = 0; cl_uint mapCount = 0; cl_mem clAssociatedMemObject = static_cast(this->associatedMemObject); @@ -169,15 +170,16 @@ cl_int MemObj::getMemObjectInfo(cl_mem_info paramName, break; case CL_MEM_OFFSET: + clOffset = this->getOffset(); if (nullptr != this->associatedMemObject) { if (this->getContext()->getBufferPoolAllocator().isPoolBuffer(this->associatedMemObject)) { - offset = 0; + clOffset = 0; } else { - offset -= this->associatedMemObject->getOffset(); + clOffset -= this->associatedMemObject->getOffset(); } } - srcParamSize = sizeof(offset); - srcParam = &offset; + srcParamSize = sizeof(clOffset); + srcParam = &clOffset; break; case CL_MEM_ASSOCIATED_MEMOBJECT: diff --git a/opencl/test/unit_test/mem_obj/buffer_pool_alloc_tests.cpp b/opencl/test/unit_test/mem_obj/buffer_pool_alloc_tests.cpp index 3e2ec93762f0d..483099b7da039 100644 --- a/opencl/test/unit_test/mem_obj/buffer_pool_alloc_tests.cpp +++ b/opencl/test/unit_test/mem_obj/buffer_pool_alloc_tests.cpp @@ -698,28 +698,48 @@ TEST_F(AggregatedSmallBuffersSubBufferApiTest, givenBufferFromPoolWhenCreateSubB } TEST_F(AggregatedSmallBuffersSubBufferApiTest, givenSubBufferFromBufferPoolWhenGetMemObjInfoCalledThenReturnValuesLikeForNormalSubBuffer) { - cl_mem buffer = clCreateBuffer(clContext, flags, size, hostPtr, &retVal); - EXPECT_EQ(retVal, CL_SUCCESS); - EXPECT_NE(buffer, nullptr); - MockBuffer *mockBuffer = static_cast(buffer); - EXPECT_TRUE(context->getBufferPoolAllocator().isPoolBuffer(mockBuffer->associatedMemObject)); + cl_mem buffer{}; + cl_mem buffer1 = clCreateBuffer(clContext, flags, size, hostPtr, &retVal); + EXPECT_EQ(CL_SUCCESS, retVal); + EXPECT_NE(nullptr, buffer1); + auto mockBuffer1 = static_cast(buffer1); + EXPECT_TRUE(context->getBufferPoolAllocator().isPoolBuffer(mockBuffer1->associatedMemObject)); + + // need buffer to have non-zero offset, to verify offset calculations in clGemMemObjectInfo + // so if we get first pool buffer with offset 0, use a second buffer + if (mockBuffer1->getOffset() != 0u) { + buffer = buffer1; + } else { + cl_mem buffer2 = clCreateBuffer(clContext, flags, size, hostPtr, &retVal); + EXPECT_EQ(CL_SUCCESS, retVal); + EXPECT_NE(nullptr, buffer2); + auto mockBuffer2 = static_cast(buffer2); + EXPECT_TRUE(context->getBufferPoolAllocator().isPoolBuffer(mockBuffer2->associatedMemObject)); + EXPECT_NE(0u, mockBuffer2->getOffset()); + buffer = buffer2; + retVal = clReleaseMemObject(buffer1); + EXPECT_EQ(retVal, CL_SUCCESS); + } cl_buffer_region region{}; region.size = 1; region.origin = size / 2; cl_mem subBuffer = clCreateSubBuffer(buffer, flags, CL_BUFFER_CREATE_TYPE_REGION, ®ion, &retVal); - EXPECT_EQ(retVal, CL_SUCCESS); - EXPECT_NE(subBuffer, nullptr); + EXPECT_EQ(CL_SUCCESS, retVal); + EXPECT_NE(nullptr, subBuffer); cl_mem associatedMemObj = nullptr; retVal = clGetMemObjectInfo(subBuffer, CL_MEM_ASSOCIATED_MEMOBJECT, sizeof(cl_mem), &associatedMemObj, nullptr); - EXPECT_EQ(retVal, CL_SUCCESS); + EXPECT_EQ(CL_SUCCESS, retVal); EXPECT_EQ(associatedMemObj, buffer); + auto mockSubBuffer = static_cast(subBuffer); + const auto offsetInternal = mockSubBuffer->getOffset(); size_t offset = 0u; retVal = clGetMemObjectInfo(subBuffer, CL_MEM_OFFSET, sizeof(size_t), &offset, nullptr); - EXPECT_EQ(retVal, CL_SUCCESS); - EXPECT_EQ(offset, region.origin); + EXPECT_EQ(CL_SUCCESS, retVal); + EXPECT_EQ(region.origin, offset); + EXPECT_EQ(offsetInternal, mockSubBuffer->getOffset()); // internal offset should not be modified after call retVal = clReleaseMemObject(subBuffer); EXPECT_EQ(retVal, CL_SUCCESS); @@ -727,7 +747,7 @@ TEST_F(AggregatedSmallBuffersSubBufferApiTest, givenSubBufferFromBufferPoolWhenG retVal = clReleaseMemObject(buffer); EXPECT_EQ(retVal, CL_SUCCESS); - EXPECT_EQ(clReleaseContext(context), CL_SUCCESS); + EXPECT_EQ(CL_SUCCESS, clReleaseContext(context)); } TEST_F(AggregatedSmallBuffersSubBufferApiTest, givenBufferFromPoolWhenCreateSubBufferCalledWithRegionOutsideBufferThenItFails) { diff --git a/opencl/test/unit_test/mem_obj/get_mem_object_info_subbuffer_tests.cpp b/opencl/test/unit_test/mem_obj/get_mem_object_info_subbuffer_tests.cpp index a750ded227141..a9baa6f0dad06 100644 --- a/opencl/test/unit_test/mem_obj/get_mem_object_info_subbuffer_tests.cpp +++ b/opencl/test/unit_test/mem_obj/get_mem_object_info_subbuffer_tests.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2019-2021 Intel Corporation + * Copyright (C) 2019-2024 Intel Corporation * * SPDX-License-Identifier: MIT * @@ -89,6 +89,10 @@ TEST_F(GetMemObjectSubBufferInfo, GivenMemOffsetWhenGettingMembojectInfoThenCorr retVal = subBuffer->getMemObjectInfo(CL_MEM_OFFSET, sizeof(offset), &offset, nullptr); EXPECT_EQ(CL_SUCCESS, retVal); EXPECT_EQ(region.origin, offset); + + retVal = subBuffer->getMemObjectInfo(CL_MEM_OFFSET, sizeof(offset), &offset, nullptr); + EXPECT_EQ(CL_SUCCESS, retVal); + EXPECT_EQ(region.origin, offset); } TEST_F(GetMemObjectSubBufferInfo, GivenMemFlagsWhenGettingMembojectInfoThenCorrectValueIsReturned) {