fix(ocl): querying memobj offset

Querying buffer offset via clGetMemObjectInfo should not modify its value. Related-To: NEO-9690 Signed-off-by: Dominik Dabek <[email protected]>
intel · Jan 19, 2024 · 7a09c51 · 7a09c51
1 parent 134c718
commit 7a09c51
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 17 deletions.
diff --git 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<cl_mem>(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
@@ -698,36 +698,56 @@ 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<MockBuffer *>(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<MockBuffer *>(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<MockBuffer *>(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, &region, &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<MockBuffer *>(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);
 
     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
@@ -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) {