Skip to content

[SYCL][AsyncAlloc] Fix minor async alloc issues #17689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 31, 2025

Conversation

npmiller
Copy link
Contributor

@npmiller npmiller commented Mar 27, 2025

This patch fixes a couple static analysis issues with the recent async alloc patch:

  • 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

@npmiller npmiller requested a review from a team as a code owner March 27, 2025 16:32
@npmiller npmiller requested a review from againull March 27, 2025 16:32
This patch fixes a couple static analysis issues with the recent async
alloc patch:

* 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
@npmiller
Copy link
Contributor Author

@intel/llvm-gatekeepers this should be ready to merge, the Jenkins/Precommit issue is unrelated and can be seen in other PRs

@sommerlukas sommerlukas merged commit a8d7f08 into intel:sycl Mar 31, 2025
32 of 35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants