Refactor Buffer::create

- simplify conditions by separation
- reorder some calls
- pass allocateMemory to AllocationFlags

Change-Id: Iab6fd35f9b9c10bfbc19058e69058346ef87dad8

runtime/mem_obj/buffer.cpp

@@ -146,31 +146,30 @@ Buffer *Buffer::create(Context *context,
         zeroCopyAllowed = false;
     }
 
+    if (allocateMemory && context->isProvidingPerformanceHints()) {
+        context->providePerformanceHint(CL_CONTEXT_DIAGNOSTICS_LEVEL_GOOD_INTEL, CL_BUFFER_NEEDS_ALLOCATE_MEMORY);
+    }
+
     if (!memory) {
-        AllocationFlags allocFlags = MemObjHelper::getAllocationFlags(flags);
+        AllocationFlags allocFlags = MemObjHelper::getAllocationFlags(flags, allocateMemory);
         DevicesBitfield devices = MemObjHelper::getDevicesBitfield(flags);
-        allocFlags.flags.allocateMemory = allocateMemory;
         memory = memoryManager->allocateGraphicsMemoryInPreferredPool(allocFlags, devices, hostPtr, static_cast<size_t>(size), allocationType);
     }
 
-    if (allocateMemory) {
+    if (allocateMemory && memory && MemoryPool::isSystemMemoryPool(memory->getMemoryPool())) {
-        if (memory) {
         memoryManager->addAllocationToHostPtrManager(memory);
     }
-        if (context->isProvidingPerformanceHints()) {
+
-            context->providePerformanceHint(CL_CONTEXT_DIAGNOSTICS_LEVEL_GOOD_INTEL, CL_BUFFER_NEEDS_ALLOCATE_MEMORY);
+    // if memory pointer should not be allcoated and graphics allocation is nullptr
-        }
+    // and cl_mem flags allow, create non-zerocopy buffer
-    } else {
+    if (!allocateMemory && !memory && Buffer::isReadOnlyMemoryPermittedByFlags(flags)) {
-        if (!memory && Buffer::isReadOnlyMemoryPermittedByFlags(flags)) {
         allocationType = GraphicsAllocation::AllocationType::BUFFER_HOST_MEMORY;
         zeroCopyAllowed = false;
         copyMemoryFromHostPtr = true;
-            AllocationFlags allocFlags = MemObjHelper::getAllocationFlags(flags);
+        AllocationFlags allocFlags = MemObjHelper::getAllocationFlags(flags, true);
         DevicesBitfield devices = MemObjHelper::getDevicesBitfield(flags);
-            allocFlags.flags.allocateMemory = true;
         memory = memoryManager->allocateGraphicsMemoryInPreferredPool(allocFlags, devices, nullptr, static_cast<size_t>(size), allocationType);
     }
-    }
 
     if (!memory) {
         errcodeRet = CL_OUT_OF_HOST_MEMORY;

runtime/mem_obj/mem_obj_helper.cpp

@@ -13,8 +13,8 @@ bool MemObjHelper::checkExtraMemFlagsForBuffer(cl_mem_flags flags) {
     return false;
 }
 
-AllocationFlags MemObjHelper::getAllocationFlags(cl_mem_flags flags) {
+AllocationFlags MemObjHelper::getAllocationFlags(cl_mem_flags flags, bool allocateMemory) {
-    return AllocationFlags(); // Initialized by default constructor
+    return AllocationFlags(allocateMemory);
 }
 
 DevicesBitfield MemObjHelper::getDevicesBitfield(cl_mem_flags flags) {

runtime/mem_obj/mem_obj_helper.h

@@ -28,7 +28,7 @@ class MemObjHelper {
 
     static bool checkExtraMemFlagsForBuffer(cl_mem_flags flags);
 
-    static AllocationFlags getAllocationFlags(cl_mem_flags flags);
+    static AllocationFlags getAllocationFlags(cl_mem_flags flags, bool allocateMemory);
 
     static DevicesBitfield getDevicesBitfield(cl_mem_flags flags);
 

runtime/mem_obj/pipe.cpp

@@ -48,9 +48,8 @@ Pipe *Pipe::create(Context *context,
     DEBUG_BREAK_IF(!memoryManager);
 
     while (true) {
-        AllocationFlags allocFlags = MemObjHelper::getAllocationFlags(flags);
+        AllocationFlags allocFlags = MemObjHelper::getAllocationFlags(flags, true);
         DevicesBitfield devices = MemObjHelper::getDevicesBitfield(flags);
-        allocFlags.flags.allocateMemory = true;
         auto size = static_cast<size_t>(packetSize * (maxPackets + 1) + intelPipeHeaderReservedSpace);
         GraphicsAllocation *memory = memoryManager->allocateGraphicsMemoryInPreferredPool(allocFlags, devices, nullptr, size, GraphicsAllocation::AllocationType::PIPE);
         if (!memory) {

unit_tests/mem_obj/buffer_tests.cpp

@@ -225,6 +225,34 @@ TEST(Buffer, givenNullptrPassedToBufferCreateWhenAllocationIsNotSystemMemoryPool
     EXPECT_FALSE(buffer->isMemObjZeroCopy());
 }
 
+TEST(Buffer, givenNullptrPassedToBufferCreateWhenAllocationIsNotSystemMemoryPoolThenAllocationIsNotAddedToHostPtrManager) {
+    std::unique_ptr<MockDevice> device(MockDevice::createWithNewExecutionEnvironment<MockDevice>(nullptr));
+    ::testing::NiceMock<GMockMemoryManagerFailFirstAllocation> *memoryManager = new ::testing::NiceMock<GMockMemoryManagerFailFirstAllocation>;
+
+    device->injectMemoryManager(memoryManager);
+    MockContext ctx(device.get());
+
+    auto allocateNonSystemGraphicsAllocation = [memoryManager](AllocationFlags flags, DevicesBitfield devicesBitfield, const void *hostPtr, size_t size, GraphicsAllocation::AllocationType type) -> GraphicsAllocation * {
+        auto allocation = memoryManager->allocateGraphicsMemory(size, MemoryConstants::pageSize, false, false);
+        reinterpret_cast<MemoryAllocation *>(allocation)->overrideMemoryPool(MemoryPool::SystemCpuInaccessible);
+        return allocation;
+    };
+
+    EXPECT_CALL(*memoryManager, allocateGraphicsMemoryInPreferredPool(::testing::_, ::testing::_, ::testing::_, ::testing::_, ::testing::_))
+        .WillOnce(::testing::Invoke(allocateNonSystemGraphicsAllocation));
+
+    cl_int retVal = 0;
+    cl_mem_flags flags = CL_MEM_READ_WRITE;
+
+    auto hostPtrAllocationCountBefore = memoryManager->hostPtrManager.getFragmentCount();
+    std::unique_ptr<Buffer> buffer(Buffer::create(&ctx, flags, MemoryConstants::pageSize, nullptr, retVal));
+
+    ASSERT_NE(nullptr, buffer.get());
+    auto hostPtrAllocationCountAfter = memoryManager->hostPtrManager.getFragmentCount();
+
+    EXPECT_EQ(hostPtrAllocationCountBefore, hostPtrAllocationCountAfter);
+}
+
 TEST(Buffer, givenNullptrPassedToBufferCreateWhenNoSharedContextOrRenderCompressedBuffersThenBuffersAllocationTypeIsBufferOrBufferHostMemory) {
     std::unique_ptr<MockDevice> device(MockDevice::createWithNewExecutionEnvironment<MockDevice>(nullptr));
     MockContext ctx(device.get());