[SYCL] accessor::get_pointer() should always return base pointer, even for offset accessors #4687
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
Signed-off-by: Chris Perkins <[email protected]>
Signed-off-by: Chris Perkins <[email protected]>
rolandschulz
reviewed
Oct 1, 2021
Signed-off-by: Chris Perkins <[email protected]>
cperkinsintel
changed the title
Signed-off-by: Chris Perkins <[email protected]>
Signed-off-by: Chris Perkins <[email protected]>
|
Couldn't it break some users that already rely on "incorrect" behavior? If so, probably we cannot just change it now, we need to go through deprecation process. |
|
/summary:run |
yes
What do you suggest? We can't deprecate the function because the function was there and is there. It just has the wrong behavior. I suggest that we need some other solution (e.g. clang-tidy). |
Signed-off-by: Chris Perkins <[email protected]>
|
/summary:run |
|
@intel/llvm-reviewers-runtime, ping. |
|
/summary:run |
|
@againull, ping. |
bader
approved these changes
Nov 30, 2021
| @@ -86,7 +86,7 @@ void stream_impl::flush() { | |||
| .get_access<access::mode::read_write, access::target::host_buffer>( | |||
| cgh); | |||
| cgh.host_task([=] { | |||
| printf("%s", BufHostAcc.get_pointer()); | |||
| printf("%s", &(BufHostAcc[0])); | |||
There was a problem hiding this comment.
@againull, I'm going to merge this PR. Feel free to review anyway.
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.
According to the SYCL 2020 spec get_pointer() returns a pointer to the beginning of the underlying buffer, even if it is offset. Presently our get_pointer() implementation returns an offset pointer when using an offset accessor. This PR fixes this.
The Jenkins Summary is failing because the CTS tests have not been updated for this requirement. I have a PR introducing this update on the CTS used by the CI system.