From 8fdc35bb4bee429abeeea3faffddfc1fe49bfa07 Mon Sep 17 00:00:00 2001 From: Mateusz Jablonski Date: Mon, 23 Nov 2020 13:51:25 +0000 Subject: [PATCH] Disallow compiling/building program if it has attached Kernels allow for creating kernel if the program is built for at least one device Related-To: NEO-5001 Signed-off-by: Mateusz Jablonski --- opencl/source/api/api.cpp | 12 ++ opencl/source/kernel/kernel.cpp | 2 + opencl/source/program/program.h | 17 +- .../unit_test/api/cl_build_program_tests.inl | 152 +++++++++++++++- .../api/cl_compile_program_tests.inl | 165 +++++++++++++++++- .../test/unit_test/program/program_tests.cpp | 35 ++++ 6 files changed, 380 insertions(+), 3 deletions(-) diff --git a/opencl/source/api/api.cpp b/opencl/source/api/api.cpp index 3f993747ec..c845f12970 100644 --- a/opencl/source/api/api.cpp +++ b/opencl/source/api/api.cpp @@ -1511,6 +1511,12 @@ cl_int CL_API_CALL clBuildProgram(cl_program program, retVal = validateObjects(WithCastToInternal(program, &pProgram), Program::isValidCallback(funcNotify, userData)); + if (CL_SUCCESS == retVal) { + if (pProgram->isLocked()) { + retVal = CL_INVALID_OPERATION; + } + } + ClDeviceVector deviceVector; ClDeviceVector *deviceVectorPtr = &deviceVector; @@ -1544,6 +1550,12 @@ cl_int CL_API_CALL clCompileProgram(cl_program program, retVal = validateObjects(WithCastToInternal(program, &pProgram), Program::isValidCallback(funcNotify, userData)); + if (CL_SUCCESS == retVal) { + if (pProgram->isLocked()) { + retVal = CL_INVALID_OPERATION; + } + } + ClDeviceVector deviceVector; ClDeviceVector *deviceVectorPtr = &deviceVector; diff --git a/opencl/source/kernel/kernel.cpp b/opencl/source/kernel/kernel.cpp index 0ff822ad06..be91eb5074 100644 --- a/opencl/source/kernel/kernel.cpp +++ b/opencl/source/kernel/kernel.cpp @@ -73,6 +73,7 @@ Kernel::Kernel(Program *programArg, const KernelInfo &kernelInfoArg, bool schedu kernelInfo(kernelInfoArg) { kernelDeviceInfos.resize(program->getMaxRootDeviceIndex() + 1); program->retain(); + program->retainForKernel(); imageTransformer.reset(new ImageTransformer); maxKernelWorkGroupSize = static_cast(deviceVector[0]->getSharedDeviceInfo().maxWorkGroupSize); @@ -105,6 +106,7 @@ Kernel::~Kernel() { } kernelArgHandlers.clear(); + program->releaseForKernel(); program->release(); } diff --git a/opencl/source/program/program.h b/opencl/source/program/program.h index da0d400653..af77b72f2a 100644 --- a/opencl/source/program/program.h +++ b/opencl/source/program/program.h @@ -154,7 +154,7 @@ class Program : public BaseObject<_cl_program> { size_t paramValueSize, void *paramValue, size_t *paramValueSizeRet) const; bool isBuilt() const { - return std::all_of(this->deviceBuildInfos.begin(), this->deviceBuildInfos.end(), [](auto deviceBuildInfo) { return deviceBuildInfo.second.buildStatus == CL_SUCCESS; }); + return std::any_of(this->deviceBuildInfos.begin(), this->deviceBuildInfos.end(), [](auto deviceBuildInfo) { return deviceBuildInfo.second.buildStatus == CL_SUCCESS && deviceBuildInfo.second.programBinaryType == CL_PROGRAM_BINARY_TYPE_EXECUTABLE; }); } Context &getContext() const { @@ -261,6 +261,19 @@ class Program : public BaseObject<_cl_program> { static cl_int processInputDevices(ClDeviceVector *&deviceVectorPtr, cl_uint numDevices, const cl_device_id *deviceList, const ClDeviceVector &allAvailableDevices); MOCKABLE_VIRTUAL void initInternalOptions(std::string &internalOptions) const; uint32_t getMaxRootDeviceIndex() const { return maxRootDeviceIndex; } + void retainForKernel() { + std::unique_lock lock{lockMutex}; + exposedKernels++; + } + void releaseForKernel() { + std::unique_lock lock{lockMutex}; + UNRECOVERABLE_IF(exposedKernels == 0); + exposedKernels--; + } + bool isLocked() { + std::unique_lock lock{lockMutex}; + return 0 != exposedKernels; + } protected: MOCKABLE_VIRTUAL cl_int createProgramFromBinary(const void *pBinary, size_t binarySize, ClDevice &clDevice); @@ -346,6 +359,8 @@ class Program : public BaseObject<_cl_program> { bool isBuiltIn = false; bool kernelDebugEnabled = false; uint32_t maxRootDeviceIndex = std::numeric_limits::max(); + std::mutex lockMutex; + uint32_t exposedKernels = 0; }; } // namespace NEO diff --git a/opencl/test/unit_test/api/cl_build_program_tests.inl b/opencl/test/unit_test/api/cl_build_program_tests.inl index 2b622314cf..2edd508dab 100644 --- a/opencl/test/unit_test/api/cl_build_program_tests.inl +++ b/opencl/test/unit_test/api/cl_build_program_tests.inl @@ -309,7 +309,7 @@ TEST_F(clBuildProgramTests, GivenValidCallbackInputWhenBuildProgramThenCallbackI EXPECT_EQ(CL_SUCCESS, retVal); } -TEST_F(clBuildProgramTests, givenMultiDeviceProgramWhenBuildingForInvalidDevicesInputThenInvalidDeviceErrorIsReturned) { +TEST_F(clBuildProgramTests, givenProgramWhenBuildingForInvalidDevicesInputThenInvalidDeviceErrorIsReturned) { cl_program pProgram = nullptr; size_t sourceSize = 0; std::string testFile; @@ -383,4 +383,154 @@ TEST_F(clBuildProgramTests, givenMultiDeviceProgramWhenBuildingForInvalidDevices EXPECT_EQ(CL_SUCCESS, retVal); } +TEST(clBuildProgramTest, givenMultiDeviceProgramWithCreatedKernelWhenBuildingThenInvalidOperationErrorIsReturned) { + + MockSpecializedContext context; + cl_program pProgram = nullptr; + size_t sourceSize = 0; + cl_int retVal = CL_INVALID_PROGRAM; + std::string testFile; + + testFile.append(clFiles); + testFile.append("copybuffer.cl"); + auto pSource = loadDataFromFile( + testFile.c_str(), + sourceSize); + + ASSERT_NE(0u, sourceSize); + ASSERT_NE(nullptr, pSource); + + const char *sources[1] = {pSource.get()}; + pProgram = clCreateProgramWithSource( + &context, + 1, + sources, + &sourceSize, + &retVal); + + EXPECT_NE(nullptr, pProgram); + ASSERT_EQ(CL_SUCCESS, retVal); + + cl_device_id firstSubDevice = context.pSubDevice0; + cl_device_id secondSubDevice = context.pSubDevice1; + + retVal = clBuildProgram( + pProgram, + 1, + &firstSubDevice, + nullptr, + nullptr, + nullptr); + + EXPECT_EQ(CL_SUCCESS, retVal); + + auto kernel = clCreateKernel(pProgram, "fullCopy", &retVal); + + EXPECT_EQ(CL_SUCCESS, retVal); + + retVal = clBuildProgram( + pProgram, + 1, + &secondSubDevice, + nullptr, + nullptr, + nullptr); + + EXPECT_EQ(CL_INVALID_OPERATION, retVal); + + retVal = clReleaseKernel(kernel); + EXPECT_EQ(CL_SUCCESS, retVal); + + retVal = clBuildProgram( + pProgram, + 1, + &secondSubDevice, + nullptr, + nullptr, + nullptr); + + EXPECT_EQ(CL_SUCCESS, retVal); + + retVal = clReleaseProgram(pProgram); + EXPECT_EQ(CL_SUCCESS, retVal); +} + +TEST(clBuildProgramTest, givenMultiDeviceProgramWithCreatedKernelsWhenBuildingThenInvalidOperationErrorIsReturned) { + + MockSpecializedContext context; + cl_program pProgram = nullptr; + size_t sourceSize = 0; + cl_int retVal = CL_INVALID_PROGRAM; + std::string testFile; + + testFile.append(clFiles); + testFile.append("copybuffer.cl"); + auto pSource = loadDataFromFile( + testFile.c_str(), + sourceSize); + + ASSERT_NE(0u, sourceSize); + ASSERT_NE(nullptr, pSource); + + const char *sources[1] = {pSource.get()}; + pProgram = clCreateProgramWithSource( + &context, + 1, + sources, + &sourceSize, + &retVal); + + EXPECT_NE(nullptr, pProgram); + ASSERT_EQ(CL_SUCCESS, retVal); + + cl_device_id firstSubDevice = context.pSubDevice0; + cl_device_id secondSubDevice = context.pSubDevice1; + + retVal = clBuildProgram( + pProgram, + 1, + &firstSubDevice, + nullptr, + nullptr, + nullptr); + + EXPECT_EQ(CL_SUCCESS, retVal); + + size_t numKernels = 0; + retVal = clGetProgramInfo(pProgram, CL_PROGRAM_NUM_KERNELS, sizeof(numKernels), &numKernels, nullptr); + EXPECT_EQ(CL_SUCCESS, retVal); + auto kernels = std::make_unique(numKernels); + + retVal = clCreateKernelsInProgram(pProgram, static_cast(numKernels), kernels.get(), nullptr); + + EXPECT_EQ(CL_SUCCESS, retVal); + + retVal = clBuildProgram( + pProgram, + 1, + &secondSubDevice, + nullptr, + nullptr, + nullptr); + + EXPECT_EQ(CL_INVALID_OPERATION, retVal); + + for (auto i = 0u; i < numKernels; i++) { + retVal = clReleaseKernel(kernels[i]); + EXPECT_EQ(CL_SUCCESS, retVal); + } + + retVal = clBuildProgram( + pProgram, + 1, + &secondSubDevice, + nullptr, + nullptr, + nullptr); + + EXPECT_EQ(CL_SUCCESS, retVal); + + retVal = clReleaseProgram(pProgram); + EXPECT_EQ(CL_SUCCESS, retVal); +} } // namespace ULT diff --git a/opencl/test/unit_test/api/cl_compile_program_tests.inl b/opencl/test/unit_test/api/cl_compile_program_tests.inl index eb74225449..52100a633a 100644 --- a/opencl/test/unit_test/api/cl_compile_program_tests.inl +++ b/opencl/test/unit_test/api/cl_compile_program_tests.inl @@ -235,7 +235,7 @@ TEST_F(clCompileProgramTests, GivenValidCallbackInputWhenLinkProgramThenCallback EXPECT_EQ(CL_SUCCESS, retVal); } -TEST(clCompileProgramTest, givenMultiDeviceProgramWhenCompilingForInvalidDevicesInputThenInvalidDeviceErrorIsReturned) { +TEST(clCompileProgramTest, givenProgramWhenCompilingForInvalidDevicesInputThenInvalidDeviceErrorIsReturned) { cl_program pProgram = nullptr; std::unique_ptr pSource = nullptr; size_t sourceSize = 0; @@ -328,4 +328,167 @@ TEST(clCompileProgramTest, givenMultiDeviceProgramWhenCompilingForInvalidDevices retVal = clReleaseProgram(pProgram); EXPECT_EQ(CL_SUCCESS, retVal); } + +TEST(clCompileProgramTest, givenMultiDeviceProgramWithCreatedKernelWhenCompilingThenInvalidOperationErrorIsReturned) { + + MockSpecializedContext context; + cl_program pProgram = nullptr; + size_t sourceSize = 0; + cl_int retVal = CL_INVALID_PROGRAM; + std::string testFile; + + testFile.append(clFiles); + testFile.append("copybuffer.cl"); + auto pSource = loadDataFromFile( + testFile.c_str(), + sourceSize); + + ASSERT_NE(0u, sourceSize); + ASSERT_NE(nullptr, pSource); + + const char *sources[1] = {pSource.get()}; + pProgram = clCreateProgramWithSource( + &context, + 1, + sources, + &sourceSize, + &retVal); + + EXPECT_NE(nullptr, pProgram); + ASSERT_EQ(CL_SUCCESS, retVal); + + cl_device_id firstSubDevice = context.pSubDevice0; + cl_device_id secondSubDevice = context.pSubDevice1; + + retVal = clBuildProgram( + pProgram, + 1, + &firstSubDevice, + nullptr, + nullptr, + nullptr); + + EXPECT_EQ(CL_SUCCESS, retVal); + + auto kernel = clCreateKernel(pProgram, "fullCopy", &retVal); + + EXPECT_EQ(CL_SUCCESS, retVal); + + retVal = clCompileProgram( + pProgram, + 1, + &secondSubDevice, + nullptr, + 0, + nullptr, + nullptr, + nullptr, + nullptr); + + EXPECT_EQ(CL_INVALID_OPERATION, retVal); + + retVal = clReleaseKernel(kernel); + EXPECT_EQ(CL_SUCCESS, retVal); + + retVal = clCompileProgram( + pProgram, + 1, + &secondSubDevice, + nullptr, + 0, + nullptr, + nullptr, + nullptr, + nullptr); + + EXPECT_EQ(CL_SUCCESS, retVal); + + retVal = clReleaseProgram(pProgram); + EXPECT_EQ(CL_SUCCESS, retVal); +} + +TEST(clCompileProgramTest, givenMultiDeviceProgramWithCreatedKernelsWhenCompilingThenInvalidOperationErrorIsReturned) { + + MockSpecializedContext context; + cl_program pProgram = nullptr; + size_t sourceSize = 0; + cl_int retVal = CL_INVALID_PROGRAM; + std::string testFile; + + testFile.append(clFiles); + testFile.append("copybuffer.cl"); + auto pSource = loadDataFromFile( + testFile.c_str(), + sourceSize); + + ASSERT_NE(0u, sourceSize); + ASSERT_NE(nullptr, pSource); + + const char *sources[1] = {pSource.get()}; + pProgram = clCreateProgramWithSource( + &context, + 1, + sources, + &sourceSize, + &retVal); + + EXPECT_NE(nullptr, pProgram); + ASSERT_EQ(CL_SUCCESS, retVal); + + cl_device_id firstSubDevice = context.pSubDevice0; + cl_device_id secondSubDevice = context.pSubDevice1; + + retVal = clBuildProgram( + pProgram, + 1, + &firstSubDevice, + nullptr, + nullptr, + nullptr); + + EXPECT_EQ(CL_SUCCESS, retVal); + + size_t numKernels = 0; + retVal = clGetProgramInfo(pProgram, CL_PROGRAM_NUM_KERNELS, sizeof(numKernels), &numKernels, nullptr); + EXPECT_EQ(CL_SUCCESS, retVal); + auto kernels = std::make_unique(numKernels); + + retVal = clCreateKernelsInProgram(pProgram, static_cast(numKernels), kernels.get(), nullptr); + + EXPECT_EQ(CL_SUCCESS, retVal); + + retVal = clCompileProgram( + pProgram, + 1, + &secondSubDevice, + nullptr, + 0, + nullptr, + nullptr, + nullptr, + nullptr); + + EXPECT_EQ(CL_INVALID_OPERATION, retVal); + + for (auto i = 0u; i < numKernels; i++) { + retVal = clReleaseKernel(kernels[i]); + EXPECT_EQ(CL_SUCCESS, retVal); + } + + retVal = clCompileProgram( + pProgram, + 1, + &secondSubDevice, + nullptr, + 0, + nullptr, + nullptr, + nullptr, + nullptr); + + EXPECT_EQ(CL_SUCCESS, retVal); + + retVal = clReleaseProgram(pProgram); + EXPECT_EQ(CL_SUCCESS, retVal); +} } // namespace ULT diff --git a/opencl/test/unit_test/program/program_tests.cpp b/opencl/test/unit_test/program/program_tests.cpp index fb57f1eae1..41cb6020e3 100644 --- a/opencl/test/unit_test/program/program_tests.cpp +++ b/opencl/test/unit_test/program/program_tests.cpp @@ -3062,3 +3062,38 @@ TEST(BuildProgramTest, givenMultiDeviceProgramWhenBuildingThenStoreAndProcessBin retVal = clReleaseProgram(pProgram); EXPECT_EQ(CL_SUCCESS, retVal); } + +TEST(ProgramTest, whenProgramIsBuiltAsAnExecutableForAtLeastOneDeviceThenIsBuiltMethodReturnsTrue) { + MockSpecializedContext context; + MockProgram program(&context, false, context.getDevices()); + EXPECT_FALSE(program.isBuilt()); + program.deviceBuildInfos[context.getDevice(0)].buildStatus = CL_BUILD_SUCCESS; + program.deviceBuildInfos[context.getDevice(0)].programBinaryType = CL_PROGRAM_BINARY_TYPE_COMPILED_OBJECT; + program.deviceBuildInfos[context.getDevice(1)].buildStatus = CL_BUILD_ERROR; + EXPECT_FALSE(program.isBuilt()); + program.deviceBuildInfos[context.getDevice(0)].buildStatus = CL_BUILD_SUCCESS; + program.deviceBuildInfos[context.getDevice(0)].programBinaryType = CL_PROGRAM_BINARY_TYPE_EXECUTABLE; + EXPECT_TRUE(program.isBuilt()); +} + +TEST(ProgramTest, givenUnlockedProgramWhenRetainForKernelIsCalledThenProgramIsLocked) { + MockSpecializedContext context; + MockProgram program(&context, false, context.getDevices()); + EXPECT_FALSE(program.isLocked()); + program.retainForKernel(); + EXPECT_TRUE(program.isLocked()); +} + +TEST(ProgramTest, givenLockedProgramWhenReleasingForKernelIsCalledForEachRetainThenProgramIsUnlocked) { + MockSpecializedContext context; + MockProgram program(&context, false, context.getDevices()); + EXPECT_FALSE(program.isLocked()); + program.retainForKernel(); + EXPECT_TRUE(program.isLocked()); + program.retainForKernel(); + EXPECT_TRUE(program.isLocked()); + program.releaseForKernel(); + EXPECT_TRUE(program.isLocked()); + program.releaseForKernel(); + EXPECT_FALSE(program.isLocked()); +}