Skip to content

Commit a8d7f08

Browse files
authored
[SYCL][AsyncAlloc] Fix minor async alloc issues (#17689)
This patch fixes a couple static analysis issues with the recent [async alloc patch](#16900): * Use `std::move` for shared pointer in `CGAsyncAlloc` constructor. It is already passed by-value to the constructor so we can just move it when assigning it to the member. * Assert that the queue is available in `AsyncFree` * Catch any exceptions from the memory pool destructor * Initialize AsyncAlloc fields in handler * Add `[[maybe_unused]]` for parameter only used in assert
1 parent fb58e23 commit a8d7f08

File tree

5 files changed

+13
-8
lines changed

5 files changed

+13
-8
lines changed

sycl/source/detail/cg.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,7 @@ class CGAsyncAlloc : public CG {
686686
ur_event_handle_t event, CG::StorageInitHelper CGData,
687687
detail::code_location loc = {})
688688
: CG(CGType::AsyncAlloc, std::move(CGData), std::move(loc)), MSize(size),
689-
MMemPool(MemPool), MEvent(event) {}
689+
MMemPool(std::move(MemPool)), MEvent(event) {}
690690

691691
std::shared_ptr<ext::oneapi::experimental::detail::memory_pool_impl>
692692
getMemPool() const {

sycl/source/detail/context_impl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ void context_impl::verifyProps(const property_list &Props) const {
567567
std::shared_ptr<sycl::ext::oneapi::experimental::detail::memory_pool_impl>
568568
context_impl::get_default_memory_pool(const context &Context,
569569
const device &Device,
570-
const usm::alloc &Kind) {
570+
[[maybe_unused]] const usm::alloc &Kind) {
571571

572572
assert(Kind == usm::alloc::device);
573573

sycl/source/detail/handler_impl.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,8 @@ class handler_impl {
207207
/// potential logging.
208208
/// Event computed from async alloc which is passed through for processing.
209209
std::shared_ptr<ext::oneapi::experimental::detail::memory_pool_impl> MMemPool;
210-
size_t MAllocSize;
211-
ur_event_handle_t MAsyncAllocEvent;
210+
size_t MAllocSize = 0;
211+
ur_event_handle_t MAsyncAllocEvent = nullptr;
212212

213213
// Allocation ptr to be freed asynchronously.
214214
void *MFreePtr = nullptr;

sycl/source/detail/memory_pool_impl.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,14 @@ memory_pool_impl::~memory_pool_impl() {
123123
if (MIsDefaultPool)
124124
return;
125125

126-
ur_usm_pool_handle_t handle = this->get_handle();
127-
sycl::context ctx = this->get_context();
128-
sycl::device dev = this->get_device();
129-
destroy_memory_pool(ctx, dev, handle);
126+
try {
127+
ur_usm_pool_handle_t handle = this->get_handle();
128+
sycl::context ctx = this->get_context();
129+
sycl::device dev = this->get_device();
130+
destroy_memory_pool(ctx, dev, handle);
131+
} catch (std::exception &e) {
132+
__SYCL_REPORT_EXCEPTION_TO_STREAM("exception in ~memory_pool_impl", e);
133+
}
130134
}
131135

132136
size_t memory_pool_impl::get_threshold() const {

sycl/source/detail/scheduler/commands.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3750,6 +3750,7 @@ ur_result_t ExecCGCommand::enqueueImpQueue() {
37503750
return UR_RESULT_SUCCESS;
37513751
}
37523752
case CGType::AsyncFree: {
3753+
assert(MQueue && "Async free submissions should have an associated queue");
37533754
CGAsyncFree *AsyncFree = (CGAsyncFree *)MCommandGroup.get();
37543755
const detail::AdapterPtr &Adapter = MQueue->getAdapter();
37553756
void *ptr = AsyncFree->getPtr();

0 commit comments

Comments
 (0)