From 89645731f6c2c357e556e0297273934d8b91e989 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Tue, 28 Mar 2023 18:36:57 -0700 Subject: [PATCH 1/8] [SYCL] Retain build-log when program build failed We should retain the build log when the program build failed. The build log is retrieved by SYCL RT. Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 01a111fbb79de..1fa8e24f0a577 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -3852,10 +3852,6 @@ pi_result piProgramBuild(pi_program Program, pi_uint32 NumDevices, // RT calls piProgramRelease(). Program->State = _pi_program::Invalid; Result = mapError(ZeResult); - if (Program->ZeBuildLog) { - ZE_CALL_NOCHECK(zeModuleBuildLogDestroy, (Program->ZeBuildLog)); - Program->ZeBuildLog = nullptr; - } if (ZeModule) { ZE_CALL_NOCHECK(zeModuleDestroy, (ZeModule)); ZeModule = nullptr; From 6635f365501d61d5d5c7f2e17e1ae87c39706d89 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Wed, 29 Mar 2023 10:42:35 -0700 Subject: [PATCH 2/8] clean up build log after RT retrives it Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 1fa8e24f0a577..d2c5ab4d12447 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -3919,6 +3919,17 @@ pi_result piProgramGetBuildInfo(pi_program Program, pi_device Device, if (ParamValueSizeRet) { *ParamValueSizeRet = LogSize; } + + // When the program build fails in piProgramBuild(), we delayed cleaning up the build log + // because RT later calls this routine to get the failed build log. + // To avoid memory leaks, we should clean up the failed build log here because + // RT does not create sycl::program when piProgramBuild() fails, thus + // it won't call piProgramRelease() to clean up the build log. + if (Program->State == _pi_program::Invalid) { + ZE_CALL_NOCHECK(zeModuleBuildLogDestroy, (Program->ZeBuildLog)); + Program->ZeBuildLog = nullptr; + } + return PI_SUCCESS; } From d6c78011fef36fe062648c2317788201660a11d1 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Mon, 3 Apr 2023 11:36:27 -0700 Subject: [PATCH 3/8] move Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 40c0558f8a672..481e92129fb50 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -3918,16 +3918,15 @@ pi_result piProgramGetBuildInfo(pi_program Program, pi_device Device, (Program->ZeBuildLog, &LogSize, pi_cast(ParamValue))); if (ParamValueSizeRet) { *ParamValueSizeRet = LogSize; - } - - // When the program build fails in piProgramBuild(), we delayed cleaning up the build log - // because RT later calls this routine to get the failed build log. - // To avoid memory leaks, we should clean up the failed build log here because - // RT does not create sycl::program when piProgramBuild() fails, thus - // it won't call piProgramRelease() to clean up the build log. - if (Program->State == _pi_program::Invalid) { - ZE_CALL_NOCHECK(zeModuleBuildLogDestroy, (Program->ZeBuildLog)); - Program->ZeBuildLog = nullptr; + // When the program build fails in piProgramBuild(), we delayed cleaning up the build log + // because RT later calls this routine to get the failed build log. + // To avoid memory leaks, we should clean up the failed build log here because + // RT does not create sycl::program when piProgramBuild() fails, thus + // it won't call piProgramRelease() to clean up the build log. + if (Program->State == _pi_program::Invalid) { + ZE_CALL_NOCHECK(zeModuleBuildLogDestroy, (Program->ZeBuildLog)); + Program->ZeBuildLog = nullptr; + } } return PI_SUCCESS; From 74d80faaa2ee417c03dd77544c33ecd392561825 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Mon, 3 Apr 2023 14:15:36 -0700 Subject: [PATCH 4/8] fix clang-format Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 481e92129fb50..3fd676d8d63cc 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -3918,11 +3918,12 @@ pi_result piProgramGetBuildInfo(pi_program Program, pi_device Device, (Program->ZeBuildLog, &LogSize, pi_cast(ParamValue))); if (ParamValueSizeRet) { *ParamValueSizeRet = LogSize; - // When the program build fails in piProgramBuild(), we delayed cleaning up the build log - // because RT later calls this routine to get the failed build log. - // To avoid memory leaks, we should clean up the failed build log here because - // RT does not create sycl::program when piProgramBuild() fails, thus - // it won't call piProgramRelease() to clean up the build log. + // When the program build fails in piProgramBuild(), we delayed cleaning + // up the build log because RT later calls this routine to get the + // failed build log. + // To avoid memory leaks, we should clean up the failed build log here + // because RT does not create sycl::program when piProgramBuild() fails, + // thus it won't call piProgramRelease() to clean up the build log. if (Program->State == _pi_program::Invalid) { ZE_CALL_NOCHECK(zeModuleBuildLogDestroy, (Program->ZeBuildLog)); Program->ZeBuildLog = nullptr; From 93c38b19667e74cb6b956e93dbfce72950e927e9 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Tue, 4 Apr 2023 11:26:00 -0700 Subject: [PATCH 5/8] check the condition Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 3fd676d8d63cc..08eeb056b51bb 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -3918,6 +3918,8 @@ pi_result piProgramGetBuildInfo(pi_program Program, pi_device Device, (Program->ZeBuildLog, &LogSize, pi_cast(ParamValue))); if (ParamValueSizeRet) { *ParamValueSizeRet = LogSize; + } + if (ParamValue) { // When the program build fails in piProgramBuild(), we delayed cleaning // up the build log because RT later calls this routine to get the // failed build log. @@ -3929,7 +3931,6 @@ pi_result piProgramGetBuildInfo(pi_program Program, pi_device Device, Program->ZeBuildLog = nullptr; } } - return PI_SUCCESS; } From f5349c26e2de701c920552012f73dda6bb62f24d Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Wed, 5 Apr 2023 15:54:05 -0700 Subject: [PATCH 6/8] call piPrgramRelease after build failure Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 12 ------------ .../detail/program_manager/program_manager.cpp | 9 ++++++--- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 08eeb056b51bb..2816e4596f162 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -3919,18 +3919,6 @@ pi_result piProgramGetBuildInfo(pi_program Program, pi_device Device, if (ParamValueSizeRet) { *ParamValueSizeRet = LogSize; } - if (ParamValue) { - // When the program build fails in piProgramBuild(), we delayed cleaning - // up the build log because RT later calls this routine to get the - // failed build log. - // To avoid memory leaks, we should clean up the failed build log here - // because RT does not create sycl::program when piProgramBuild() fails, - // thus it won't call piProgramRelease() to clean up the build log. - if (Program->State == _pi_program::Invalid) { - ZE_CALL_NOCHECK(zeModuleBuildLogDestroy, (Program->ZeBuildLog)); - Program->ZeBuildLog = nullptr; - } - } return PI_SUCCESS; } diff --git a/sycl/source/detail/program_manager/program_manager.cpp b/sycl/source/detail/program_manager/program_manager.cpp index 7fdf942cd2d16..9fff8c5e3a1fa 100644 --- a/sycl/source/detail/program_manager/program_manager.cpp +++ b/sycl/source/detail/program_manager/program_manager.cpp @@ -1108,9 +1108,12 @@ ProgramManager::build(ProgramPtr Program, const ContextImplPtr Context, RT::PiResult Error = Plugin.call_nocheck( Program.get(), /*num devices =*/1, &Device, Options.c_str(), nullptr, nullptr); - if (Error != PI_SUCCESS) - throw compile_program_error(getProgramBuildLog(Program.get(), Context), - Error); + if (Error != PI_SUCCESS) { + std::string BuildLog = getProgramBuildLog(Program.get(), Context); + // PiProgram is still alive in plugin, so we need to release the memory for it + Plugin.call_nocheck(Program.get()); + throw compile_program_error(BuildLog, Error); + } return Program; } From eaa9c9924deec78e2488454d122ff7d0deb1b299 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Wed, 5 Apr 2023 16:00:11 -0700 Subject: [PATCH 7/8] fix clang-format Signed-off-by: Byoungro So --- sycl/source/detail/program_manager/program_manager.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sycl/source/detail/program_manager/program_manager.cpp b/sycl/source/detail/program_manager/program_manager.cpp index 9fff8c5e3a1fa..931a1069378b0 100644 --- a/sycl/source/detail/program_manager/program_manager.cpp +++ b/sycl/source/detail/program_manager/program_manager.cpp @@ -1110,7 +1110,8 @@ ProgramManager::build(ProgramPtr Program, const ContextImplPtr Context, nullptr); if (Error != PI_SUCCESS) { std::string BuildLog = getProgramBuildLog(Program.get(), Context); - // PiProgram is still alive in plugin, so we need to release the memory for it + // PiProgram is still alive in plugin, so we need to release the memory + // for it Plugin.call_nocheck(Program.get()); throw compile_program_error(BuildLog, Error); } From e6c8b1a08ed4b65035e561ca8ef01bd595a35294 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Wed, 5 Apr 2023 20:29:38 -0700 Subject: [PATCH 8/8] try to revert the latest change Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 12 ++++++++++++ .../detail/program_manager/program_manager.cpp | 10 +++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 2816e4596f162..08eeb056b51bb 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -3919,6 +3919,18 @@ pi_result piProgramGetBuildInfo(pi_program Program, pi_device Device, if (ParamValueSizeRet) { *ParamValueSizeRet = LogSize; } + if (ParamValue) { + // When the program build fails in piProgramBuild(), we delayed cleaning + // up the build log because RT later calls this routine to get the + // failed build log. + // To avoid memory leaks, we should clean up the failed build log here + // because RT does not create sycl::program when piProgramBuild() fails, + // thus it won't call piProgramRelease() to clean up the build log. + if (Program->State == _pi_program::Invalid) { + ZE_CALL_NOCHECK(zeModuleBuildLogDestroy, (Program->ZeBuildLog)); + Program->ZeBuildLog = nullptr; + } + } return PI_SUCCESS; } diff --git a/sycl/source/detail/program_manager/program_manager.cpp b/sycl/source/detail/program_manager/program_manager.cpp index 931a1069378b0..7fdf942cd2d16 100644 --- a/sycl/source/detail/program_manager/program_manager.cpp +++ b/sycl/source/detail/program_manager/program_manager.cpp @@ -1108,13 +1108,9 @@ ProgramManager::build(ProgramPtr Program, const ContextImplPtr Context, RT::PiResult Error = Plugin.call_nocheck( Program.get(), /*num devices =*/1, &Device, Options.c_str(), nullptr, nullptr); - if (Error != PI_SUCCESS) { - std::string BuildLog = getProgramBuildLog(Program.get(), Context); - // PiProgram is still alive in plugin, so we need to release the memory - // for it - Plugin.call_nocheck(Program.get()); - throw compile_program_error(BuildLog, Error); - } + if (Error != PI_SUCCESS) + throw compile_program_error(getProgramBuildLog(Program.get(), Context), + Error); return Program; }