Skip to content
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

[SYCL][CUDA] Change hint functions to return UR_SUCCESS #11296

Closed

Conversation

fabiomestre
Copy link
Contributor

@fabiomestre fabiomestre commented Sep 25, 2023

  • The UR spec was recently changed to make hint entryponts
    always return UR_RESULT_SUCCESS. This commit changes the CUDA
    adapter to be conformant with this change.
  • This commit also changes the type of PointerRangeSize which
    was causing a stack corruption.

@fabiomestre fabiomestre temporarily deployed to WindowsCILock September 25, 2023 16:02 — with GitHub Actions Inactive
@fabiomestre fabiomestre force-pushed the fabio/cuda_update_hint_ep branch from c6f3316 to da555b6 Compare September 25, 2023 16:03
- The UR spec was recently changed to make hint entryponts
always return UR_RESULT_SUCCESS. This commit changes the CUDA
adapter to be conformant with this change.
- This commit also changes the type of PointerRangeSize which
 was causing a stack corruption.
@fabiomestre fabiomestre force-pushed the fabio/cuda_update_hint_ep branch from da555b6 to 02dd7ba Compare September 25, 2023 16:04
@fabiomestre fabiomestre changed the title [SYCL][CUDA] Change hint functions to return UR_SUCCESS when the hint is ignored [SYCL][CUDA] Change hint functions to return UR_SUCCESS Sep 25, 2023
@fabiomestre fabiomestre temporarily deployed to WindowsCILock September 25, 2023 16:06 — with GitHub Actions Inactive
@jinz2014
Copy link
Contributor

I have a question. The deleted message in "setErrorMessage()" may be informative for users. If they are deleted, will a user still know about the cause of lack of support ?

@fabiomestre
Copy link
Contributor Author

I have a question. The deleted message in "setErrorMessage()" may be informative for users. If they are deleted, will a user still know about the cause of lack of support ?

That's a good point.

The aim of this PR is to make all the adapters have consistent behaviours. Since performance hints don't have an impact in the behaviour of a program, it's usually an accepted pattern that such functions should return success even when the performance hint is ignored.

The current behaviour of the CUDA adapter returning an error in this situations is an exception to this. Maybe, in the future, this could be turned into warnings if support for them is added to the UR specification (oneapi-src/unified-runtime#762).

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.

2 participants