From f62375d36c2cc87567f6a3c18c170011100e45b1 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Wed, 22 Dec 2021 12:38:15 -0800 Subject: [PATCH 01/12] [SYCL][L0] return error code when ZE_DEBUG=4 detects memory leaks. Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 9a7c09418d6b7..ee94337efdb36 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -7421,6 +7421,7 @@ pi_result piextPluginGetOpaqueData(void *opaque_data_param, // the plugin is unloaded from memory. pi_result piTearDown(void *PluginParameter) { (void)PluginParameter; + bool LeakFound = false; // reclaim pi_platform objects here since we don't have piPlatformRelease. for (pi_platform &Platform : *PiPlatformsCache) { delete Platform; @@ -7498,8 +7499,10 @@ pi_result piTearDown(void *PluginParameter) { fprintf(stderr, "%30s = %-5d", ZeName, ZeCount); } - if (diff) + if (diff) { + LeakFound = true; fprintf(stderr, " ---> LEAK = %d", diff); + } fprintf(stderr, "\n"); } @@ -7507,6 +7510,8 @@ pi_result piTearDown(void *PluginParameter) { delete ZeCallCount; ZeCallCount = nullptr; } + if (LeakFound) + return PI_INVALID_MEM_OBJECT; return PI_SUCCESS; } From e04b730a3f4d462f550fb3fc6d9a7dd7081f3475 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Thu, 10 Feb 2022 12:49:55 -0800 Subject: [PATCH 02/12] [SYCL][L0] Remove ZeModule when program build failed When a sycl::program is attempted to build, a ZeModule is created. When the attempt failed, we need to clean up the ZeModule that is associated with the failed program to avoid memory leak. Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index dda8ae72e976a..09a04e5bbefa6 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -4306,8 +4306,14 @@ pi_result piProgramBuild(pi_program Program, pi_uint32 NumDevices, // check now for unresolved symbols. ze_result_t ZeResult = checkUnresolvedSymbols(ZeModule, &Program->ZeBuildLog); if (ZeResult == ZE_RESULT_ERROR_MODULE_LINK_FAILURE) { + // remove ZeModule that is associated with the failed program + ZE_CALL_NOCHECK(zeModuleDestroy, (ZeModule)); + ZeModule = nullptr; return PI_BUILD_PROGRAM_FAILURE; } else if (ZeResult != ZE_RESULT_SUCCESS) { + // remove ZeModule that is associated with the failed program + ZE_CALL_NOCHECK(zeModuleDestroy, (ZeModule)); + ZeModule = nullptr; return mapError(ZeResult); } From 9a6b91d2f6cad69a7694e19ea360d512808190f5 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Fri, 11 Feb 2022 09:45:05 -0800 Subject: [PATCH 03/12] restructure Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 09a04e5bbefa6..3d2915962ef76 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -4305,15 +4305,12 @@ pi_result piProgramBuild(pi_program Program, pi_uint32 NumDevices, // are supposed to be fully linked and ready to use. Therefore, do an extra // check now for unresolved symbols. ze_result_t ZeResult = checkUnresolvedSymbols(ZeModule, &Program->ZeBuildLog); - if (ZeResult == ZE_RESULT_ERROR_MODULE_LINK_FAILURE) { - // remove ZeModule that is associated with the failed program - ZE_CALL_NOCHECK(zeModuleDestroy, (ZeModule)); - ZeModule = nullptr; - return PI_BUILD_PROGRAM_FAILURE; - } else if (ZeResult != ZE_RESULT_SUCCESS) { + if (ZeResult != ZE_RESULT_SUCCESS) { // remove ZeModule that is associated with the failed program ZE_CALL_NOCHECK(zeModuleDestroy, (ZeModule)); - ZeModule = nullptr; + + if (ZeResult == ZE_RESULT_ERROR_MODULE_LINK_FAILURE) + return PI_BUILD_PROGRAM_FAILURE; return mapError(ZeResult); } From d6c6193aa99b0dc287779a7bfb2bf823d123f592 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Fri, 11 Feb 2022 09:51:25 -0800 Subject: [PATCH 04/12] Update sycl/plugins/level_zero/pi_level_zero.cpp Co-authored-by: smaslov-intel --- sycl/plugins/level_zero/pi_level_zero.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 3d2915962ef76..051c25a04d2e2 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -4307,7 +4307,7 @@ pi_result piProgramBuild(pi_program Program, pi_uint32 NumDevices, ze_result_t ZeResult = checkUnresolvedSymbols(ZeModule, &Program->ZeBuildLog); if (ZeResult != ZE_RESULT_SUCCESS) { // remove ZeModule that is associated with the failed program - ZE_CALL_NOCHECK(zeModuleDestroy, (ZeModule)); + ZE_CALL(zeModuleDestroy, (ZeModule)); if (ZeResult == ZE_RESULT_ERROR_MODULE_LINK_FAILURE) return PI_BUILD_PROGRAM_FAILURE; From b605f0100a5c58f846e7ecb7cb3b7fb171d6b8b7 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Fri, 11 Feb 2022 12:16:31 -0800 Subject: [PATCH 05/12] set zeModule nullptr to avoid double destroy Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 3d2915962ef76..81d19f2a3098a 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -4308,7 +4308,9 @@ pi_result piProgramBuild(pi_program Program, pi_uint32 NumDevices, if (ZeResult != ZE_RESULT_SUCCESS) { // remove ZeModule that is associated with the failed program ZE_CALL_NOCHECK(zeModuleDestroy, (ZeModule)); - + // Also set Program->ZeModule nullptr to avoid double destroy of zeModule in + // case where SYCL RT calls piProgramRelease(). + Program->ZeModule = nullptr; if (ZeResult == ZE_RESULT_ERROR_MODULE_LINK_FAILURE) return PI_BUILD_PROGRAM_FAILURE; return mapError(ZeResult); From c3157ce2bd0f8a55c15df666508ab3b34459b176 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Fri, 11 Feb 2022 13:32:04 -0800 Subject: [PATCH 06/12] address feedback Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 5100bc5d7f551..a84c02575de27 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -4296,8 +4296,9 @@ pi_result piProgramBuild(pi_program Program, pi_uint32 NumDevices, ze_device_handle_t ZeDevice = DeviceList[0]->ZeDevice; ze_context_handle_t ZeContext = Program->Context->ZeContext; ze_module_handle_t ZeModule = nullptr; - ze_result_t ZeResult = ZE_CALL(zeModuleCreate, (ZeContext, ZeDevice, &ZeModuleDesc, &ZeModule, - &Program->ZeBuildLog)); + ze_result_t ZeResult = + ZE_CALL_NOCHECK(zeModuleCreate, (ZeContext, ZeDevice, &ZeModuleDesc, + &ZeModule, &Program->ZeBuildLog)); if (ZeResult != ZE_RESULT_SUCCESS) { // We need to clear Program state to avoid double destroy of zeModule in // case where SYCL RT calls piProgramRelease(). @@ -4311,7 +4312,7 @@ pi_result piProgramBuild(pi_program Program, pi_uint32 NumDevices, // call to zeModuleDynamicLink. However, modules created with piProgramBuild // are supposed to be fully linked and ready to use. Therefore, do an extra // check now for unresolved symbols. - ze_result_t ZeResult = checkUnresolvedSymbols(ZeModule, &Program->ZeBuildLog); + ZeResult = checkUnresolvedSymbols(ZeModule, &Program->ZeBuildLog); if (ZeResult != ZE_RESULT_SUCCESS) { // remove ZeModule that is associated with the failed program ZE_CALL(zeModuleDestroy, (ZeModule)); From ae223478c8d8253513cbc7f55ddb5d83ce0f7de8 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Thu, 17 Feb 2022 16:23:10 -0800 Subject: [PATCH 07/12] removed call to ZeModuleDestroy Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index a84c02575de27..570b51d11fe88 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -4300,12 +4300,12 @@ pi_result piProgramBuild(pi_program Program, pi_uint32 NumDevices, ZE_CALL_NOCHECK(zeModuleCreate, (ZeContext, ZeDevice, &ZeModuleDesc, &ZeModule, &Program->ZeBuildLog)); if (ZeResult != ZE_RESULT_SUCCESS) { - // We need to clear Program state to avoid double destroy of zeModule in - // case where SYCL RT calls piProgramRelease(). - // We should not return with an error code here due to the comments below. + // We adjust pi_program below to avoid attempting to release zeModule when + // RT calls piProgramRelease(). Program->ZeModule = nullptr; Program->Code.reset(); Program->State = _pi_program::Invalid; + return mapError(ZeResult); } // The call to zeModuleCreate does not report an error if there are // unresolved symbols because it thinks these could be resolved later via a @@ -4314,9 +4314,9 @@ pi_result piProgramBuild(pi_program Program, pi_uint32 NumDevices, // check now for unresolved symbols. ZeResult = checkUnresolvedSymbols(ZeModule, &Program->ZeBuildLog); if (ZeResult != ZE_RESULT_SUCCESS) { - // remove ZeModule that is associated with the failed program - ZE_CALL(zeModuleDestroy, (ZeModule)); - + // Note that the ZeModule is still allocated and will be released when + // the user catch the exception that RT throws. + // Otherwise, the user program crashes and memory leak is not a concern. if (ZeResult == ZE_RESULT_ERROR_MODULE_LINK_FAILURE) return PI_BUILD_PROGRAM_FAILURE; return mapError(ZeResult); From 9a8b9075c577c0a6e4549dfeea255abbeab08044 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Fri, 18 Feb 2022 11:05:02 -0800 Subject: [PATCH 08/12] Update sycl/plugins/level_zero/pi_level_zero.cpp Co-authored-by: smaslov-intel --- sycl/plugins/level_zero/pi_level_zero.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 570b51d11fe88..f3a3ffc6d3386 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -4316,7 +4316,7 @@ pi_result piProgramBuild(pi_program Program, pi_uint32 NumDevices, if (ZeResult != ZE_RESULT_SUCCESS) { // Note that the ZeModule is still allocated and will be released when // the user catch the exception that RT throws. - // Otherwise, the user program crashes and memory leak is not a concern. + // Otherwise, the user program terminates and memory leak is not a concern. if (ZeResult == ZE_RESULT_ERROR_MODULE_LINK_FAILURE) return PI_BUILD_PROGRAM_FAILURE; return mapError(ZeResult); From 28d6f10526148a09eb76ecbb784fef983834026a Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Fri, 18 Feb 2022 11:38:00 -0800 Subject: [PATCH 09/12] added binding of ZeModule to Program Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 570b51d11fe88..376a35807bfc3 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -4313,21 +4313,20 @@ pi_result piProgramBuild(pi_program Program, pi_uint32 NumDevices, // are supposed to be fully linked and ready to use. Therefore, do an extra // check now for unresolved symbols. ZeResult = checkUnresolvedSymbols(ZeModule, &Program->ZeBuildLog); + // We no longer need the IL / native code. + Program->Code.reset(); + Program->ZeModule = ZeModule; if (ZeResult != ZE_RESULT_SUCCESS) { // Note that the ZeModule is still allocated and will be released when // the user catch the exception that RT throws. // Otherwise, the user program crashes and memory leak is not a concern. + Program->State = _pi_program::Invalid; if (ZeResult == ZE_RESULT_ERROR_MODULE_LINK_FAILURE) return PI_BUILD_PROGRAM_FAILURE; return mapError(ZeResult); } - // We no longer need the IL / native code. - Program->Code.reset(); - - Program->ZeModule = ZeModule; Program->State = _pi_program::Exe; - return PI_SUCCESS; } From a8fdd7613dedbc36d377e6576efd712266707cf4 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Fri, 18 Feb 2022 13:13:45 -0800 Subject: [PATCH 10/12] address feedback Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index beef664ab5f66..e1093968760ef 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -4296,6 +4296,9 @@ pi_result piProgramBuild(pi_program Program, pi_uint32 NumDevices, ze_device_handle_t ZeDevice = DeviceList[0]->ZeDevice; ze_context_handle_t ZeContext = Program->Context->ZeContext; ze_module_handle_t ZeModule = nullptr; + + pi_result Result = PI_SUCCESS; + Program->State = _pi_program::Exe; ze_result_t ZeResult = ZE_CALL_NOCHECK(zeModuleCreate, (ZeContext, ZeDevice, &ZeModuleDesc, &ZeModule, &Program->ZeBuildLog)); @@ -4303,7 +4306,6 @@ pi_result piProgramBuild(pi_program Program, pi_uint32 NumDevices, // We adjust pi_program below to avoid attempting to release zeModule when // RT calls piProgramRelease(). Program->ZeModule = nullptr; - Program->Code.reset(); Program->State = _pi_program::Invalid; return mapError(ZeResult); } @@ -4313,20 +4315,17 @@ pi_result piProgramBuild(pi_program Program, pi_uint32 NumDevices, // are supposed to be fully linked and ready to use. Therefore, do an extra // check now for unresolved symbols. ZeResult = checkUnresolvedSymbols(ZeModule, &Program->ZeBuildLog); - // We no longer need the IL / native code. - Program->Code.reset(); - Program->ZeModule = ZeModule; if (ZeResult != ZE_RESULT_SUCCESS) { - // Note that the ZeModule is still allocated and will be released when - // the user catch the exception that RT throws. - // Otherwise, the user program terminates and memory leak is not a concern. - if (ZeResult == ZE_RESULT_ERROR_MODULE_LINK_FAILURE) - return PI_BUILD_PROGRAM_FAILURE; - return mapError(ZeResult); + Program->State = _pi_program::Invalid; + Result = (ZeResult == ZE_RESULT_ERROR_MODULE_LINK_FAILURE) + ? PI_BUILD_PROGRAM_FAILURE + : mapError(ZeResult); } - Program->State = _pi_program::Exe; - return PI_SUCCESS; + // We no longer need the IL / native code. + Program->Code.reset(); + Program->ZeModule = ZeModule; + return Result; } pi_result piProgramGetBuildInfo(pi_program Program, pi_device Device, From ad98494c57714886e3df0e67f8238bace8e46c43 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Fri, 18 Feb 2022 15:58:20 -0800 Subject: [PATCH 11/12] set Result Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index e1093968760ef..debf0f64f75ca 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -4307,7 +4307,7 @@ pi_result piProgramBuild(pi_program Program, pi_uint32 NumDevices, // RT calls piProgramRelease(). Program->ZeModule = nullptr; Program->State = _pi_program::Invalid; - return mapError(ZeResult); + Result = mapError(ZeResult); } // The call to zeModuleCreate does not report an error if there are // unresolved symbols because it thinks these could be resolved later via a From a1b1000f56ca2fe94967f4d9d05cd117859cf41b Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Fri, 18 Feb 2022 16:09:40 -0800 Subject: [PATCH 12/12] reorg Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 27 ++++++++++++----------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index debf0f64f75ca..12579b0769b45 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -4305,21 +4305,22 @@ pi_result piProgramBuild(pi_program Program, pi_uint32 NumDevices, if (ZeResult != ZE_RESULT_SUCCESS) { // We adjust pi_program below to avoid attempting to release zeModule when // RT calls piProgramRelease(). - Program->ZeModule = nullptr; + ZeModule = nullptr; Program->State = _pi_program::Invalid; Result = mapError(ZeResult); - } - // The call to zeModuleCreate does not report an error if there are - // unresolved symbols because it thinks these could be resolved later via a - // call to zeModuleDynamicLink. However, modules created with piProgramBuild - // are supposed to be fully linked and ready to use. Therefore, do an extra - // check now for unresolved symbols. - ZeResult = checkUnresolvedSymbols(ZeModule, &Program->ZeBuildLog); - if (ZeResult != ZE_RESULT_SUCCESS) { - Program->State = _pi_program::Invalid; - Result = (ZeResult == ZE_RESULT_ERROR_MODULE_LINK_FAILURE) - ? PI_BUILD_PROGRAM_FAILURE - : mapError(ZeResult); + } else { + // The call to zeModuleCreate does not report an error if there are + // unresolved symbols because it thinks these could be resolved later via a + // call to zeModuleDynamicLink. However, modules created with + // piProgramBuild are supposed to be fully linked and ready to use. + // Therefore, do an extra check now for unresolved symbols. + ZeResult = checkUnresolvedSymbols(ZeModule, &Program->ZeBuildLog); + if (ZeResult != ZE_RESULT_SUCCESS) { + Program->State = _pi_program::Invalid; + Result = (ZeResult == ZE_RESULT_ERROR_MODULE_LINK_FAILURE) + ? PI_BUILD_PROGRAM_FAILURE + : mapError(ZeResult); + } } // We no longer need the IL / native code.