-
Notifications
You must be signed in to change notification settings - Fork 730
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[SYCL] Fix resource leak related to SYCL_FALLBACK_ASSERT (#12532)
#6837 enabled asynchronous buffer destruction for buffers constructed without host data. However, initial fallback assert implementation in #3767 predates it and as such had to place the buffer inside `queue_impl` to avoid unintended synchronization point. I don't know if there was the same crash observed on the end-to-end test added as part of this PR prior to #3767, but it doesn't even matter because the "new" implementation is both simpler and doesn't result in a crash. I suspect that without it (with the buffer for fallback assert implementation being a data member of `sycl::queue_impl`) we had a cyclic dependency somewhere leading to resource leak and ultimately to the assert in `DeviceGlobalUSMMem::~DeviceGlobalUSMMem()`.
- Loading branch information
1 parent
7348207
commit b478d2f
Showing
4 changed files
with
49 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
// RUN: %{build} -o %t.out | ||
// RUN: %{run} %t.out | ||
|
||
// Device globals aren't supported on opencl:gpu yet. | ||
// UNSUPPORTED: opencl && gpu | ||
|
||
// TODO: Fails at JIT compilation for some reason. | ||
// UNSUPPORTED: hip | ||
#define SYCL_FALLBACK_ASSERT 1 | ||
|
||
#include <sycl/sycl.hpp> | ||
|
||
// DeviceGlobalUSMMem::~DeviceGlobalUSMMem() has asserts to ensure some | ||
// resources have been cleaned up when it's executed. Those asserts used to fail | ||
// when "AssertHappened" buffer used in fallback implementation of the device | ||
// assert was a data member of the queue_impl. | ||
sycl::ext::oneapi::experimental::device_global<int32_t> dg; | ||
|
||
int main() { | ||
sycl::queue q; | ||
q.submit([&](sycl::handler &cgh) { | ||
sycl::range<1> R{16}; | ||
cgh.parallel_for(sycl::nd_range<1>{R, R}, [=](sycl::nd_item<1> ndi) { | ||
if (ndi.get_global_linear_id() == 0) | ||
dg.get() = 42; | ||
auto sg = sycl::ext::oneapi::experimental::this_sub_group(); | ||
auto active = sycl::ext::oneapi::group_ballot(sg, 1); | ||
}); | ||
}).wait(); | ||
|
||
return 0; | ||
} |