From 8ec7e45bffc35db70a091b694f6282ac7c00cb37 Mon Sep 17 00:00:00 2001 From: Jaroslaw Chodor Date: Sun, 20 Oct 2019 14:22:34 +0200 Subject: [PATCH] [2/N] compiler interface refactor (spir/spirV fix) Fixing regression in spir/spirV scenarios Resolves: NEO-3854, NEO-3852, NEO-3845, NEO-3851, NEO-3844 Change-Id: Ifee7f1847c7f6598428f96ae2241b3d85e58ad11 --- Jenkinsfile | 2 +- runtime/program/build.cpp | 19 ++++++----- runtime/program/compile.cpp | 5 +++ runtime/program/create.inl | 1 - runtime/program/get_info.cpp | 6 ++-- runtime/program/process_spir_binary.cpp | 4 +-- .../program/process_spir_binary_tests.cpp | 6 ++-- unit_tests/program/program_tests.cpp | 33 +++++++++++++++++++ 8 files changed, 58 insertions(+), 18 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 593da88960..ea642f8ff9 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -1,5 +1,5 @@ #!groovy dependenciesRevision='3fca422c0e381f607f9c8c02dca161010c30dac8-1319' strategy='EQUAL' -allowedCD=260 +allowedCD=259 allowedF=7 diff --git a/runtime/program/build.cpp b/runtime/program/build.cpp index b180b2c020..a7559b10aa 100644 --- a/runtime/program/build.cpp +++ b/runtime/program/build.cpp @@ -69,13 +69,15 @@ cl_int Program::build( break; } - auto inType = IGC::CodeType::oclC; + TranslationInput inputArgs = {IGC::CodeType::oclC, IGC::CodeType::oclGenBin}; if ((createdFrom == CreatedFrom::IL) || (this->programBinaryType == CL_PROGRAM_BINARY_TYPE_INTERMEDIATE)) { - inType = isSpirV ? IGC::CodeType::spirV : IGC::CodeType::llvmBc; + inputArgs.srcType = isSpirV ? IGC::CodeType::spirV : IGC::CodeType::llvmBc; + inputArgs.src = ArrayRef(irBinary.get(), irBinarySize); + } else { + inputArgs.src = ArrayRef(sourceCode.c_str(), sourceCode.size()); } - TranslationInput inputArgs = {inType, IGC::CodeType::oclGenBin}; - if (strcmp(sourceCode.c_str(), "") == 0) { + if (inputArgs.src.size() == 0) { retVal = CL_INVALID_PROGRAM; break; } @@ -95,7 +97,6 @@ cl_int Program::build( internalOptions.append(compilerExtensionsOptions); } - inputArgs.src = ArrayRef(sourceCode.c_str(), sourceCode.size()); inputArgs.apiOptions = ArrayRef(options.c_str(), options.length()); inputArgs.internalOptions = ArrayRef(internalOptions.c_str(), internalOptions.length()); inputArgs.GTPinInput = gtpinGetIgcInit(); @@ -114,9 +115,11 @@ cl_int Program::build( if (retVal != CL_SUCCESS) { break; } - this->irBinary = std::move(compilerOuput.intermediateRepresentation.mem); - this->irBinarySize = compilerOuput.intermediateRepresentation.size; - this->isSpirV = compilerOuput.intermediateCodeType == IGC::CodeType::spirV; + if (inputArgs.srcType == IGC::CodeType::oclC) { + this->irBinary = std::move(compilerOuput.intermediateRepresentation.mem); + this->irBinarySize = compilerOuput.intermediateRepresentation.size; + this->isSpirV = compilerOuput.intermediateCodeType == IGC::CodeType::spirV; + } this->genBinary = std::move(compilerOuput.deviceBinary.mem); this->genBinarySize = compilerOuput.deviceBinary.size; this->debugData = std::move(compilerOuput.debugData.mem); diff --git a/runtime/program/compile.cpp b/runtime/program/compile.cpp index 027cc34a31..c46bcf0a36 100644 --- a/runtime/program/compile.cpp +++ b/runtime/program/compile.cpp @@ -70,6 +70,11 @@ cl_int Program::compile( break; } + if ((createdFrom == CreatedFrom::IL) || (this->programBinaryType == CL_PROGRAM_BINARY_TYPE_INTERMEDIATE)) { + retVal = CL_SUCCESS; + break; + } + buildStatus = CL_BUILD_IN_PROGRESS; options = (buildOptions != nullptr) ? buildOptions : ""; diff --git a/runtime/program/create.inl b/runtime/program/create.inl index fddccfcc92..94a6ddf83b 100644 --- a/runtime/program/create.inl +++ b/runtime/program/create.inl @@ -158,7 +158,6 @@ T *Program::createFromIL(Context *ctx, T *program = new T(*ctx->getDevice(0)->getExecutionEnvironment(), ctx, false); errcodeRet = program->createProgramFromBinary(il, length); - program->createdFrom = CreatedFrom::IL; if (errcodeRet != CL_SUCCESS) { diff --git a/runtime/program/get_info.cpp b/runtime/program/get_info.cpp index 7070fd1e92..b4c45a30f0 100644 --- a/runtime/program/get_info.cpp +++ b/runtime/program/get_info.cpp @@ -104,14 +104,14 @@ cl_int Program::getInfo(cl_program_info paramName, size_t paramValueSize, break; case CL_PROGRAM_IL: - pSrc = sourceCode.data(); - retSize = srcSize = sourceCode.size(); - if (!Program::isValidSpirvBinary(pSrc, srcSize)) { + if (createdFrom != CreatedFrom::IL) { if (paramValueSizeRet) { *paramValueSizeRet = 0; } return CL_SUCCESS; } + pSrc = irBinary.get(); + retSize = srcSize = irBinarySize; break; case CL_PROGRAM_DEBUG_INFO_SIZES_INTEL: diff --git a/runtime/program/process_spir_binary.cpp b/runtime/program/process_spir_binary.cpp index a59bb4de66..0912c1cf7b 100644 --- a/runtime/program/process_spir_binary.cpp +++ b/runtime/program/process_spir_binary.cpp @@ -31,8 +31,8 @@ cl_int Program::processSpirBinary( bool isSpirV) { programBinaryType = CL_PROGRAM_BINARY_TYPE_INTERMEDIATE; - std::string binaryString(static_cast(pBinary), binarySize); - sourceCode.swap(binaryString); + this->irBinary = makeCopy(pBinary, binarySize); + this->irBinarySize = binarySize; buildStatus = CL_BUILD_NONE; this->isSpirV = isSpirV; diff --git a/unit_tests/program/process_spir_binary_tests.cpp b/unit_tests/program/process_spir_binary_tests.cpp index 0f6224be5e..9065ee3e44 100644 --- a/unit_tests/program/process_spir_binary_tests.cpp +++ b/unit_tests/program/process_spir_binary_tests.cpp @@ -37,7 +37,7 @@ TEST_F(ProcessSpirBinaryTests, InvalidSizeBinary) { auto retVal = program->processSpirBinary(pBinary, binarySize, false); EXPECT_EQ(CL_SUCCESS, retVal); - EXPECT_EQ(binarySize, program->sourceCode.size()); + EXPECT_EQ(binarySize, program->irBinarySize); } TEST_F(ProcessSpirBinaryTests, SomeBinary) { @@ -46,8 +46,8 @@ TEST_F(ProcessSpirBinaryTests, SomeBinary) { auto retVal = program->processSpirBinary(pBinary, binarySize, false); EXPECT_EQ(CL_SUCCESS, retVal); - EXPECT_EQ(0, strcmp(pBinary, program->sourceCode.c_str())); - EXPECT_EQ(binarySize, program->sourceCode.size()); + EXPECT_EQ(0, memcmp(pBinary, program->irBinary.get(), program->irBinarySize)); + EXPECT_EQ(binarySize, program->irBinarySize); // Verify no built log is available auto pBuildLog = program->getBuildLog(program->getDevicePtr()); diff --git a/unit_tests/program/program_tests.cpp b/unit_tests/program/program_tests.cpp index e0a037f414..f644f79bc4 100644 --- a/unit_tests/program/program_tests.cpp +++ b/unit_tests/program/program_tests.cpp @@ -2642,6 +2642,39 @@ TEST_F(ProgramTests, createFromILWhenCreateProgramFromBinaryIsSuccessfulThenRetu prog->release(); } +TEST_F(ProgramTests, givenProgramCreatedFromILWhenCompileIsCalledThenReuseTheILInsteadOfCallingCompilerInterface) { + const uint32_t spirv[16] = {0x03022307}; + cl_int errCode = 0; + auto prog = Program::createFromIL(pContext, reinterpret_cast(spirv), sizeof(spirv), errCode); + ASSERT_NE(nullptr, prog); + cl_device_id deviceId = pDevice; + auto debugVars = NEO::getIgcDebugVars(); + debugVars.forceBuildFailure = true; + gEnvironment->fclPushDebugVars(debugVars); + auto compilerErr = prog->compile(1, &deviceId, nullptr, 0, nullptr, nullptr, nullptr, nullptr); + EXPECT_EQ(CL_SUCCESS, compilerErr); + gEnvironment->fclPopDebugVars(); + prog->release(); +} + +TEST_F(ProgramTests, givenProgramCreatedFromIntermediateBinaryRepresentationWhenCompileIsCalledThenReuseTheILInsteadOfCallingCompilerInterface) { + const uint32_t spirv[16] = {0x03022307}; + cl_int errCode = 0; + cl_device_id deviceId = pDevice; + cl_context ctx = pContext; + size_t lengths = sizeof(spirv); + const unsigned char *binaries[1] = {reinterpret_cast(spirv)}; + auto prog = Program::create(ctx, 1U, &deviceId, &lengths, binaries, nullptr, errCode); + ASSERT_NE(nullptr, prog); + auto debugVars = NEO::getIgcDebugVars(); + debugVars.forceBuildFailure = true; + gEnvironment->fclPushDebugVars(debugVars); + auto compilerErr = prog->compile(1, &deviceId, nullptr, 0, nullptr, nullptr, nullptr, nullptr); + EXPECT_EQ(CL_SUCCESS, compilerErr); + gEnvironment->fclPopDebugVars(); + prog->release(); +} + TEST_F(ProgramTests, createFromILWhenIlIsNullptrThenReturnsInvalidBinaryError) { cl_int errCode = CL_SUCCESS; constexpr cl_int expectedErrCode = CL_INVALID_BINARY;