[SYCL][CUDA] Using Custom context by default #1742
Merged
Conversation
This file contains hidden or 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
romanovvlad
reviewed
May 25, 2020
sycl/plugins/cuda/pi_cuda.cpp
Outdated
| _pi_event::make_native(PI_COMMAND_TYPE_MEM_BUFFER_MAP, command_queue); | ||
| (*retEvent)->record(); | ||
| try { | ||
| assert(command_queue); |
There was a problem hiding this comment.
Suggest moving it to the begging of the function(~line 3342).
There was a problem hiding this comment.
Good call. I have moved this assert to the beginning of the function.
sycl/source/detail/queue_impl.hpp
Outdated
| enum class cuda_context_type : char { primary, custom }; | ||
|
|
||
| /// Default context type created for CUDA backend | ||
| const cuda_context_type DefaultContextType = cuda_context_type::custom; |
There was a problem hiding this comment.
Suggested change
| const cuda_context_type DefaultContextType = cuda_context_type::custom; | |
| constexpr cuda_context_type DefaultContextType = cuda_context_type::custom; |
There was a problem hiding this comment.
This suggestion has been applied to the commit. Thanks!
sycl/plugins/cuda/pi_cuda.cpp
Outdated
| command_queue); | ||
| (*retEvent)->record(); | ||
| try { | ||
| assert(command_queue); |
There was a problem hiding this comment.
Suggest moving it to the begging of the function(~line 3391).
There was a problem hiding this comment.
This has also been moved to the start of its function.
Performance analysis shows is better the default behaviour of SYCL Queues on CUDA backend is to use the non-primary context. Primary context will be exposed for interop with CUDA Runtime API on a separate extension as a property. Signed-off-by: Ruyman Reyes <[email protected]>
Signed-off-by: Ruyman Reyes <[email protected]>
steffenlarsen
force-pushed
the
cuda-context-not-primary
branch
from
c71bbfc to
af5f78c
Compare
s-kanaev
approved these changes
May 27, 2020
romanovvlad
approved these changes
May 27, 2020
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.
Performance analysis shows is better that the default behavior of
SYCL Queues on CUDA backend is to use the non-primary context.
Primary context will be exposed for interop with CUDA Runtime API
on a separate extension as a property.
Signed-off-by: Ruyman Reyes [email protected]