-
Notifications
You must be signed in to change notification settings - Fork 117
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
[Exp] Added const qualifier to some parameters. #699
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,10 +10,10 @@ name: ImportExp | |
details: | ||
- "Import memory into USM" | ||
params: | ||
- type: $x_context_handle_t | ||
- type: "const $x_context_handle_t" | ||
name: hContext | ||
desc: "[in] handle of the context object" | ||
- type: "void*" | ||
- type: "const void*" | ||
name: pMem | ||
desc: "[in] pointer to host memory object" | ||
- type: "size_t" | ||
|
@@ -30,10 +30,10 @@ name: ReleaseExp | |
details: | ||
- "Release memory from USM" | ||
params: | ||
- type: $x_context_handle_t | ||
- type: "const $x_context_handle_t" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also don't think this should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm closing this PR because I don't think these changes are needed. There is a remapping, including casting, between SYCL objects and UR objects anyway. |
||
name: hContext | ||
desc: "[in] handle of the context object" | ||
- type: "void*" | ||
- type: "const void*" | ||
name: pMem | ||
desc: "[in] pointer to host memory object" | ||
returns: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the context should be
const
qualified the reason being that an adapter may wish to store data in the context relating to the action being performed in theurUSMImportExp
/urUSMExportExp
entry points, making this argumentconst
means that becomes impossible. Making thisconst
is also inconsistent with the rest of the API.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, removed that const on the UR context.
Note that at the SYCL level all USM APIs (such as malloc_device) that take a queue or context parameter declare them const. But its fine to have the underlying context modifiable.