diff --git a/Jenkinsfile b/Jenkinsfile index 371921a32b..a4f41037b7 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -1,5 +1,5 @@ #!groovy dependenciesRevision='e299ec97c8d0d14d604a94606ce11c7498e30a76-1244' strategy='EQUAL' -allowedCD=276 +allowedCD=275 allowedF=4 diff --git a/runtime/program/kernel_info.cpp b/runtime/program/kernel_info.cpp index de71a2c773..28c388cc7e 100644 --- a/runtime/program/kernel_info.cpp +++ b/runtime/program/kernel_info.cpp @@ -199,9 +199,6 @@ void WorkSizeInfo::checkRatio(const size_t workItems[3]) { KernelInfo::~KernelInfo() { kernelArgInfo.clear(); - for (auto &stringData : patchInfo.stringDataMap) { - delete[] stringData.second.pStringData; - } patchInfo.stringDataMap.clear(); delete[] crossThreadData; } @@ -376,14 +373,9 @@ void KernelInfo::storePatchToken(const SPatchAllocateStatelessDefaultDeviceQueue void KernelInfo::storePatchToken(const SPatchString *pStringArg) { uint32_t stringIndex = pStringArg->Index; - PrintfStringInfo printfStringInfo; - printfStringInfo.SizeInBytes = pStringArg->StringSize; - if (printfStringInfo.SizeInBytes) { - printfStringInfo.pStringData = new char[printfStringInfo.SizeInBytes]; - if (printfStringInfo.pStringData != nullptr) { - memcpy_s(printfStringInfo.pStringData, printfStringInfo.SizeInBytes, (cl_char *)pStringArg + sizeof(SPatchString), printfStringInfo.SizeInBytes); - patchInfo.stringDataMap.insert(std::pair(stringIndex, printfStringInfo)); - } + if (pStringArg->StringSize > 0) { + const char *stringData = reinterpret_cast(pStringArg + 1); + patchInfo.stringDataMap.emplace(stringIndex, std::string(stringData, stringData + pStringArg->StringSize)); } } @@ -404,11 +396,6 @@ void KernelInfo::storePatchToken(const SPatchAllocateSystemThreadSurface *pSyste patchInfo.pAllocateSystemThreadSurface = pSystemThreadSurface; } -const char *KernelInfo::queryPrintfString(uint32_t index) const { - auto printfInfo = patchInfo.stringDataMap.find(index); - return printfInfo == patchInfo.stringDataMap.end() ? nullptr : printfInfo->second.pStringData; -} - cl_int KernelInfo::resolveKernelInfo() { cl_int retVal = CL_SUCCESS; std::unordered_map::iterator iterUint; diff --git a/runtime/program/kernel_info.h b/runtime/program/kernel_info.h index ec7ca3a01c..cb5d08e9e5 100644 --- a/runtime/program/kernel_info.h +++ b/runtime/program/kernel_info.h @@ -163,8 +163,6 @@ struct KernelInfo { void storeKernelArgPatchInfo(uint32_t argNum, uint32_t dataSize, uint32_t crossthreadOffset, uint32_t sourceOffset, uint32_t offsetSSH); - const char *queryPrintfString(uint32_t index) const; - size_t getSamplerStateArrayCount() const; size_t getSamplerStateArraySize(const HardwareInfo &hwInfo) const; size_t getBorderColorStateSize() const; diff --git a/runtime/program/patch_info.h b/runtime/program/patch_info.h index a36372834e..22ba10f1ba 100644 --- a/runtime/program/patch_info.h +++ b/runtime/program/patch_info.h @@ -10,6 +10,8 @@ #include "patch_list.h" #include +#include +#include #include namespace NEO { @@ -44,11 +46,6 @@ using iOpenCL::SPatchString; using iOpenCL::SPatchThreadPayload; using iOpenCL::SProgramBinaryHeader; -typedef struct TagPrintfStringInfo { - size_t SizeInBytes; - char *pStringData; -} PrintfStringInfo, *PPrintfStringInfo; - struct PatchInfo { const SPatchMediaInterfaceDescriptorLoad *interfaceDescriptorDataLoad = nullptr; const SPatchAllocateLocalSurface *localsurface = nullptr; @@ -74,7 +71,7 @@ struct PatchInfo { const SPatchAllocateStatelessEventPoolSurface *pAllocateStatelessEventPoolSurface = nullptr; const SPatchAllocateStatelessDefaultDeviceQueueSurface *pAllocateStatelessDefaultDeviceQueueSurface = nullptr; const SPatchAllocateSystemThreadSurface *pAllocateSystemThreadSurface = nullptr; - ::std::map stringDataMap; + ::std::unordered_map stringDataMap; ::std::vector kernelArgumentInfo; PatchInfo() { diff --git a/runtime/program/print_formatter.cpp b/runtime/program/print_formatter.cpp index 893d88a619..88d6d371dc 100644 --- a/runtime/program/print_formatter.cpp +++ b/runtime/program/print_formatter.cpp @@ -14,27 +14,24 @@ namespace NEO { -PrintFormatter::PrintFormatter(Kernel &kernelArg, GraphicsAllocation &dataArg) : kernel(kernelArg), - data(dataArg), - buffer(nullptr), - bufferSize(0), - offset(0) { +PrintFormatter::PrintFormatter(const uint8_t *printfOutputBuffer, uint32_t printfOutputBufferMaxSize, + bool using32BitPointers, const StringMap &stringLiteralMap) + : printfOutputBuffer(printfOutputBuffer), + printfOutputBufferSize(printfOutputBufferMaxSize), + stringLiteralMap(stringLiteralMap), + using32BitPointers(using32BitPointers) { } void PrintFormatter::printKernelOutput(const std::function &print) { - offset = 0; - buffer = reinterpret_cast(data.getUnderlyingBuffer()); + currentOffset = 0; - // first 4 bytes of the buffer store it's own size - // before reading it size needs to be set to 4 because read() checks bounds and would fail if bufferSize was 0 - bufferSize = 4; - read(&bufferSize); + // first 4 bytes of the buffer store the actual size of data that was written by printf from within EUs + read(&printfOutputBufferSize); uint32_t stringIndex = 0; - - while (offset + 4 <= bufferSize) { + while (currentOffset + 4 <= printfOutputBufferSize) { read(&stringIndex); - const char *formatString = kernel.getKernelInfo().queryPrintfString(stringIndex); + const char *formatString = queryPrintfString(stringIndex); if (formatString != nullptr) { printString(formatString, print); } @@ -146,7 +143,7 @@ size_t PrintFormatter::printStringToken(char *output, size_t size, const char *f read(&type); read(&index); if (type == static_cast(PRINTF_DATA_TYPE::STRING)) { - return simple_sprintf(output, size, formatString, kernel.getKernelInfo().queryPrintfString(index)); + return simple_sprintf(output, size, formatString, queryPrintfString(index)); } else { return simple_sprintf(output, size, formatString, 0); } @@ -156,7 +153,7 @@ size_t PrintFormatter::printPointerToken(char *output, size_t size, const char * uint64_t value = {0}; read(&value); - if (kernel.is32Bit()) { + if (using32BitPointers) { value &= 0x00000000FFFFFFFF; } @@ -196,4 +193,10 @@ bool PrintFormatter::isConversionSpecifier(char c) { return false; } } + +const char *PrintFormatter::queryPrintfString(uint32_t index) const { + auto stringEntry = stringLiteralMap.find(index); + return stringEntry == stringLiteralMap.end() ? nullptr : stringEntry->second.c_str(); +} + } // namespace NEO diff --git a/runtime/program/print_formatter.h b/runtime/program/print_formatter.h index 2baa9d6405..b3e00e9267 100644 --- a/runtime/program/print_formatter.h +++ b/runtime/program/print_formatter.h @@ -15,11 +15,15 @@ #include #include #include +#include +#include extern int memcpy_s(void *dst, size_t destSize, const void *src, size_t count); namespace NEO { +using StringMap = std::unordered_map; + enum class PRINTF_DATA_TYPE : int { INVALID, BYTE, @@ -40,12 +44,14 @@ enum class PRINTF_DATA_TYPE : int { class PrintFormatter { public: - PrintFormatter(Kernel &kernelArg, GraphicsAllocation &dataArg); + PrintFormatter(const uint8_t *printfOutputBuffer, uint32_t printfOutputBufferMaxSize, + bool using32BitPointers, const StringMap &stringLiteralMap); void printKernelOutput(const std::function &print = [](char *str) { printToSTDOUT(str); }); static const size_t maxPrintfOutputLength = 1024; protected: + const char *queryPrintfString(uint32_t index) const; void printString(const char *formatString, const std::function &print); size_t printToken(char *output, size_t size, const char *formatString); size_t printStringToken(char *output, size_t size, const char *formatString); @@ -58,15 +64,15 @@ class PrintFormatter { template bool read(T *value) { - if (offset + sizeof(T) <= bufferSize) { - auto srcPtr = reinterpret_cast(buffer + offset); + if (currentOffset + sizeof(T) <= printfOutputBufferSize) { + auto srcPtr = reinterpret_cast(printfOutputBuffer + currentOffset); if (isAligned(srcPtr)) { *value = *srcPtr; } else { - memcpy_s(value, bufferSize - offset, srcPtr, sizeof(T)); + memcpy_s(value, printfOutputBufferSize - currentOffset, srcPtr, sizeof(T)); } - offset += sizeof(T); + currentOffset += sizeof(T); return true; } else { return false; @@ -101,17 +107,18 @@ class PrintFormatter { } if (sizeof(T) < 4) { - offset += (4 - sizeof(T)) * valueCount; + currentOffset += (4 - sizeof(T)) * valueCount; } return charactersPrinted; } - Kernel &kernel; - GraphicsAllocation &data; + const uint8_t *printfOutputBuffer = nullptr; // buffer extracted from the kernel, contains values to be printed + uint32_t printfOutputBufferSize = 0; // size of the data contained in the buffer - uint8_t *buffer; // buffer extracted from the kernel, contains values to be printed - uint32_t bufferSize; // size of the data contained in the buffer - uint32_t offset; // current position in currently parsed buffer + const StringMap &stringLiteralMap; + bool using32BitPointers = false; + + uint32_t currentOffset = 0; // current position in currently parsed buffer }; }; // namespace NEO diff --git a/runtime/program/printf_handler.cpp b/runtime/program/printf_handler.cpp index 44d7db6f77..2a46762888 100644 --- a/runtime/program/printf_handler.cpp +++ b/runtime/program/printf_handler.cpp @@ -58,7 +58,8 @@ void PrintfHandler::makeResident(CommandStreamReceiver &commandStreamReceiver) { } void PrintfHandler::printEnqueueOutput() { - PrintFormatter printFormatter(*kernel, *printfSurface); + PrintFormatter printFormatter(reinterpret_cast(printfSurface->getUnderlyingBuffer()), static_cast(printfSurface->getUnderlyingBufferSize()), + kernel->is32Bit(), kernel->getKernelInfo().patchInfo.stringDataMap); printFormatter.printKernelOutput(); } } // namespace NEO diff --git a/unit_tests/command_queue/enqueue_kernel_2_tests.cpp b/unit_tests/command_queue/enqueue_kernel_2_tests.cpp index f2e50c4e18..e00f5bbc93 100644 --- a/unit_tests/command_queue/enqueue_kernel_2_tests.cpp +++ b/unit_tests/command_queue/enqueue_kernel_2_tests.cpp @@ -610,14 +610,9 @@ HWTEST_P(EnqueueKernelPrintfTest, GivenKernelWithPrintfBlockedByEventWhenEventUn auto crossThreadData = reinterpret_cast(mockKernel.mockKernel->getCrossThreadData()); - char *testString = new char[sizeof("test")]; - strcpy_s(testString, sizeof("test"), "test"); + std::string testString = "test"; - PrintfStringInfo printfStringInfo; - printfStringInfo.SizeInBytes = sizeof("test"); - printfStringInfo.pStringData = testString; - - mockKernel.kernelInfo.patchInfo.stringDataMap.insert(std::make_pair(0, printfStringInfo)); + mockKernel.kernelInfo.patchInfo.stringDataMap.insert(std::make_pair(0, testString)); cl_uint workDim = 1; size_t globalWorkOffset[3] = {0, 0, 0}; diff --git a/unit_tests/event/event_tests.cpp b/unit_tests/event/event_tests.cpp index 79ceff7911..b19556d1a9 100644 --- a/unit_tests/event/event_tests.cpp +++ b/unit_tests/event/event_tests.cpp @@ -548,19 +548,13 @@ TEST_F(InternalsEventTest, givenBlockedKernelWithPrintfWhenSubmittedThenPrintOut pPrintfSurface->DataParamOffset = 0; pPrintfSurface->DataParamSize = 8; - char *testString = new char[sizeof("test")]; - - strcpy_s(testString, sizeof("test"), "test"); - - PrintfStringInfo printfStringInfo; - printfStringInfo.SizeInBytes = sizeof("test"); - printfStringInfo.pStringData = testString; + std::string testString = "test"; MockKernelWithInternals mockKernelWithInternals(*pDevice); auto pKernel = mockKernelWithInternals.mockKernel; KernelInfo *kernelInfo = const_cast(&pKernel->getKernelInfo()); kernelInfo->patchInfo.pAllocateStatelessPrintfSurface = pPrintfSurface; - kernelInfo->patchInfo.stringDataMap.insert(std::make_pair(0, printfStringInfo)); + kernelInfo->patchInfo.stringDataMap.insert(std::make_pair(0, testString)); uint64_t crossThread[10]; pKernel->setCrossThreadData(&crossThread, sizeof(uint64_t) * 8); diff --git a/unit_tests/program/kernel_data.cpp b/unit_tests/program/kernel_data.cpp index 839ff309fc..53a0e72871 100644 --- a/unit_tests/program/kernel_data.cpp +++ b/unit_tests/program/kernel_data.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2017-2018 Intel Corporation + * Copyright (C) 2017-2019 Intel Corporation * * SPDX-License-Identifier: MIT * @@ -123,7 +123,7 @@ TEST_F(KernelDataTest, PrintfString) { buildAndDecode(); - EXPECT_EQ_VAL(0, strcmp(stringValue, pKernelInfo->queryPrintfString(0))); + EXPECT_EQ_VAL(0, strcmp(stringValue, pKernelInfo->patchInfo.stringDataMap.find(0)->second.c_str())); delete[] pPrintfString; } diff --git a/unit_tests/program/printf_helper_tests.cpp b/unit_tests/program/printf_helper_tests.cpp index c914a9b70a..238e12558e 100644 --- a/unit_tests/program/printf_helper_tests.cpp +++ b/unit_tests/program/printf_helper_tests.cpp @@ -23,7 +23,7 @@ using namespace iOpenCL; // -------------------- Base Fixture ------------------------ class PrintFormatterTest : public testing::Test { public: - PrintFormatter *printFormatter; + std::unique_ptr printFormatter; std::string format; uint8_t buffer; @@ -50,7 +50,7 @@ class PrintFormatterTest : public testing::Test { program = std::make_unique(*device->getExecutionEnvironment()); kernel = new MockKernel(program.get(), *kernelInfo, *device); - printFormatter = new PrintFormatter(*kernel, *data); + printFormatter = std::unique_ptr(new PrintFormatter(static_cast(data->getUnderlyingBuffer()), PrintFormatter::maxPrintfOutputLength, is32bit, kernelInfo->patchInfo.stringDataMap)); underlyingBuffer[0] = 0; underlyingBuffer[1] = 0; @@ -59,7 +59,6 @@ class PrintFormatterTest : public testing::Test { } void TearDown() override { - delete printFormatter; delete data; delete kernel; delete device; @@ -751,6 +750,7 @@ TEST_F(PrintFormatterTest, GivenPrintfFormatWhenPointerThenInsertAddress) { } TEST_F(PrintFormatterTest, GivenPrintfFormatWhenPointerWith32BitKernelThenPrint32BitPointer) { + printFormatter.reset(new PrintFormatter(static_cast(data->getUnderlyingBuffer()), PrintFormatter::maxPrintfOutputLength, true, kernelInfo->patchInfo.stringDataMap)); auto stringIndex = injectFormatString("%p"); storeData(stringIndex); kernelInfo->gpuPointerSize = 4;