[SYCL] Remove half from global namespace #4818
Merged
Conversation
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
|
/summary:run |
bader
changed the title
romanovvlad
approved these changes
Oct 26, 2021
npmiller
added a commit
to npmiller/oneMKL
that referenced
this pull request
Nov 16, 2021
`half` has been removed from the global scope in DPC++, see intel/llvm#4818
mkrainiuk
pushed a commit
to uxlfoundation/oneMath
that referenced
this pull request
Nov 17, 2021
* [cublas] Fix race condition in cublas handle deletion This was causing segmentation faults at the end of the program when running tests. Cublas handles need to be created per thread and per context, therefore they're currently being stored in a `static thread_local` map, mapping contexts to cublas handles. These handles are deleted in two places, the map destructor called when the thread is deleted, or the context destructor, where a callback is registered on the context to delete the cublas handle. Since there is two places where a handle can be deleted and that these two will likely run in separate threads, the handle was placed in an C++ atomic to avoid race conditions, and allocated on the heap so that it can stay alive when one of either the thread or the context is deleted. But there was two issues with this, the main one is that the callback registered on the context was not given the pointer to the atomic handle, rather the pointer to the slot in the `thread_local` map where the atomic pointer was stored. Which meant that whenever the thread would be deleted before the context, the pointer available to the callback would be invalid as the map was deleted. The solution for that is to simply pass directly the pointer to the atomic handle to the callback. The second issue is that both of these places were deleting the atomic in all cases, which means one of the deletion was invalid. The solution for this is to only do the deletion in the last one being called, that is to say the one which will get a `nullptr` when atomically accessing the handle pointer. With these two patches the tests are running fine. * [cublas] Fix formatting of cublas handle patch * Fix half types for DPC++ `half` has been removed from the global scope in DPC++, see intel/llvm#4818
sbalint98
pushed a commit
to sbalint98/oneMKL
that referenced
this pull request
Dec 6, 2021
) * [cublas] Fix race condition in cublas handle deletion This was causing segmentation faults at the end of the program when running tests. Cublas handles need to be created per thread and per context, therefore they're currently being stored in a `static thread_local` map, mapping contexts to cublas handles. These handles are deleted in two places, the map destructor called when the thread is deleted, or the context destructor, where a callback is registered on the context to delete the cublas handle. Since there is two places where a handle can be deleted and that these two will likely run in separate threads, the handle was placed in an C++ atomic to avoid race conditions, and allocated on the heap so that it can stay alive when one of either the thread or the context is deleted. But there was two issues with this, the main one is that the callback registered on the context was not given the pointer to the atomic handle, rather the pointer to the slot in the `thread_local` map where the atomic pointer was stored. Which meant that whenever the thread would be deleted before the context, the pointer available to the callback would be invalid as the map was deleted. The solution for that is to simply pass directly the pointer to the atomic handle to the callback. The second issue is that both of these places were deleting the atomic in all cases, which means one of the deletion was invalid. The solution for this is to only do the deletion in the last one being called, that is to say the one which will get a `nullptr` when atomically accessing the handle pointer. With these two patches the tests are running fine. * [cublas] Fix formatting of cublas handle patch * Fix half types for DPC++ `half` has been removed from the global scope in DPC++, see intel/llvm#4818
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
API removal has been approved.