From ab74b606e95ebe0dab5b86d9d0c69f52a5d4ed4b Mon Sep 17 00:00:00 2001 From: "Chodor, Jaroslaw" Date: Fri, 12 Jul 2019 13:34:54 +0200 Subject: [PATCH] Fixing regression in progvar_prog_scope_init Change-Id: I20424ce9e0c9a295bb8b9d5608252c3d4802e9da --- runtime/program/process_gen_binary.cpp | 63 +++++++++++++---------- unit_tests/program/program_data_tests.cpp | 14 ++++- 2 files changed, 47 insertions(+), 30 deletions(-) diff --git a/runtime/program/process_gen_binary.cpp b/runtime/program/process_gen_binary.cpp index d38aab55e1..71ea7d8eba 100644 --- a/runtime/program/process_gen_binary.cpp +++ b/runtime/program/process_gen_binary.cpp @@ -881,14 +881,17 @@ GraphicsAllocation *allocateGlobalsSurface(NEO::Context *ctx, NEO::Device *devic svmProps.readOnly = constant; svmProps.hostPtrReadOnly = constant; auto ptr = ctx->getSVMAllocsManager()->createSVMAlloc(size, svmProps); - UNRECOVERABLE_IF(device == nullptr); + UNRECOVERABLE_IF(ptr == nullptr); auto gpuAlloc = ctx->getSVMAllocsManager()->getSVMAlloc(ptr)->gpuAllocation; + UNRECOVERABLE_IF(gpuAlloc == nullptr); + UNRECOVERABLE_IF(device == nullptr); device->getMemoryManager()->copyMemoryToAllocation(gpuAlloc, initData, static_cast(size)); return ctx->getSVMAllocsManager()->getSVMAlloc(ptr)->gpuAllocation; } else { UNRECOVERABLE_IF(device == nullptr); auto allocationType = constant ? GraphicsAllocation::AllocationType::CONSTANT_SURFACE : GraphicsAllocation::AllocationType::GLOBAL_SURFACE; auto gpuAlloc = device->getMemoryManager()->allocateGraphicsMemoryWithProperties({size, allocationType}); + UNRECOVERABLE_IF(gpuAlloc == nullptr); memcpy_s(gpuAlloc->getUnderlyingBuffer(), gpuAlloc->getUnderlyingBufferSize(), initData, size); return gpuAlloc; } @@ -952,26 +955,24 @@ cl_int Program::parseProgramScopePatchList() { }; break; - case PATCH_TOKEN_GLOBAL_POINTER_PROGRAM_BINARY_INFO: - if (globalSurface != nullptr) { - auto patch = *(SPatchGlobalPointerProgramBinaryInfo *)pPatch; - if ((patch.GlobalBufferIndex == 0) && (patch.BufferIndex == 0) && (patch.BufferType == PROGRAM_SCOPE_GLOBAL_BUFFER)) { - globalVariablesSelfPatches.push_back(readMisalignedUint64(&patch.GlobalPointerOffset)); - } else { - printDebugString(DebugManager.flags.PrintDebugMessages.get(), stderr, "Program::parseProgramScopePatchList. Unhandled Data parameter: %d\n", pPatch->Token); - } - DBG_LOG(LogPatchTokens, - "\n .GLOBAL_POINTER_PROGRAM_BINARY_INFO", pPatch->Token, - "\n .Size", pPatch->Size, - "\n .GlobalBufferIndex", patch.GlobalBufferIndex, - "\n .GlobalPointerOffset", patch.GlobalPointerOffset, - "\n .BufferType", patch.BufferType, - "\n .BufferIndex", patch.BufferIndex); + case PATCH_TOKEN_GLOBAL_POINTER_PROGRAM_BINARY_INFO: { + auto patch = *(SPatchGlobalPointerProgramBinaryInfo *)pPatch; + if ((patch.GlobalBufferIndex == 0) && (patch.BufferIndex == 0) && (patch.BufferType == PROGRAM_SCOPE_GLOBAL_BUFFER)) { + globalVariablesSelfPatches.push_back(readMisalignedUint64(&patch.GlobalPointerOffset)); + } else { + printDebugString(DebugManager.flags.PrintDebugMessages.get(), stderr, "Program::parseProgramScopePatchList. Unhandled Data parameter: %d\n", pPatch->Token); + } + DBG_LOG(LogPatchTokens, + "\n .GLOBAL_POINTER_PROGRAM_BINARY_INFO", pPatch->Token, + "\n .Size", pPatch->Size, + "\n .GlobalBufferIndex", patch.GlobalBufferIndex, + "\n .GlobalPointerOffset", patch.GlobalPointerOffset, + "\n .BufferType", patch.BufferType, + "\n .BufferIndex", patch.BufferIndex); } break; - case PATCH_TOKEN_CONSTANT_POINTER_PROGRAM_BINARY_INFO: - if (constantSurface != nullptr) { + case PATCH_TOKEN_CONSTANT_POINTER_PROGRAM_BINARY_INFO: { auto patch = *(SPatchConstantPointerProgramBinaryInfo *)pPatch; if ((patch.ConstantBufferIndex == 0) && (patch.BufferIndex == 0) && (patch.BufferType == PROGRAM_SCOPE_CONSTANT_BUFFER)) { globalConstantsSelfPatches.push_back(readMisalignedUint64(&patch.ConstantPointerOffset)); @@ -1021,22 +1022,28 @@ cl_int Program::parseProgramScopePatchList() { } for (auto offset : globalVariablesSelfPatches) { - UNRECOVERABLE_IF(globalSurface == nullptr); - void *pPtr = ptrOffset(globalSurface->getUnderlyingBuffer(), static_cast(offset)); - if (globalSurface->is32BitAllocation()) { - *reinterpret_cast(pPtr) += static_cast(globalSurface->getGpuAddressToPatch()); + if (globalSurface == nullptr) { + retVal = CL_INVALID_BINARY; } else { - *reinterpret_cast(pPtr) += static_cast(globalSurface->getGpuAddressToPatch()); + void *pPtr = ptrOffset(globalSurface->getUnderlyingBuffer(), static_cast(offset)); + if (globalSurface->is32BitAllocation()) { + *reinterpret_cast(pPtr) += static_cast(globalSurface->getGpuAddressToPatch()); + } else { + *reinterpret_cast(pPtr) += static_cast(globalSurface->getGpuAddressToPatch()); + } } } for (auto offset : globalConstantsSelfPatches) { - UNRECOVERABLE_IF(constantSurface == nullptr); - void *pPtr = ptrOffset(constantSurface->getUnderlyingBuffer(), static_cast(offset)); - if (constantSurface->is32BitAllocation()) { - *reinterpret_cast(pPtr) += static_cast(constantSurface->getGpuAddressToPatch()); + if (constantSurface == nullptr) { + retVal = CL_INVALID_BINARY; } else { - *reinterpret_cast(pPtr) += static_cast(constantSurface->getGpuAddressToPatch()); + void *pPtr = ptrOffset(constantSurface->getUnderlyingBuffer(), static_cast(offset)); + if (constantSurface->is32BitAllocation()) { + *reinterpret_cast(pPtr) += static_cast(constantSurface->getGpuAddressToPatch()); + } else { + *reinterpret_cast(pPtr) += static_cast(constantSurface->getGpuAddressToPatch()); + } } } diff --git a/unit_tests/program/program_data_tests.cpp b/unit_tests/program/program_data_tests.cpp index 2ac9301b2f..6c575f1efd 100644 --- a/unit_tests/program/program_data_tests.cpp +++ b/unit_tests/program/program_data_tests.cpp @@ -43,7 +43,6 @@ class ProgramDataTestBase : public testing::Test, void buildAndDecodeProgramPatchList(); - protected: void SetUp() override { PlatformFixture::SetUp(); cl_device_id device = pPlatform->getDevice(0); @@ -120,6 +119,8 @@ class ProgramDataTestBase : public testing::Test, SProgramBinaryHeader programBinaryHeader; void *pProgramPatchList; uint32_t programPatchListSize; + cl_int patchlistDecodeErrorCode = 0; + bool allowDecodeFailure = false; }; template @@ -153,7 +154,10 @@ void ProgramDataTestBase::buildAndDecodeProgramPatchList() { pProgram->storeGenBinary(pProgramData, headerSize + programBinaryHeader.PatchListSize); error = pProgram->processGenBinary(); - EXPECT_EQ(CL_SUCCESS, error); + patchlistDecodeErrorCode = error; + if (allowDecodeFailure == false) { + EXPECT_EQ(CL_SUCCESS, error); + } delete[] pProgramData; } @@ -363,8 +367,11 @@ TEST_F(ProgramDataTest, GlobalPointerProgramBinaryInfo) { pProgramPatchList = (void *)pGlobalPointer; programPatchListSize = globalPointer.Size; + this->allowDecodeFailure = true; buildAndDecodeProgramPatchList(); EXPECT_EQ(nullptr, pProgram->getGlobalSurface()); + EXPECT_EQ(CL_INVALID_BINARY, this->patchlistDecodeErrorCode); + this->allowDecodeFailure = false; delete[] pGlobalPointer; @@ -552,8 +559,11 @@ TEST_F(ProgramDataTest, ConstantPointerProgramBinaryInfo) { pProgramPatchList = (void *)pConstantPointer; programPatchListSize = constantPointer.Size; + this->allowDecodeFailure = true; buildAndDecodeProgramPatchList(); EXPECT_EQ(nullptr, pProgram->getConstantSurface()); + EXPECT_EQ(CL_INVALID_BINARY, this->patchlistDecodeErrorCode); + this->allowDecodeFailure = false; delete[] pConstantPointer;