Skip to content
This repository has been archived by the owner on Mar 28, 2023. It is now read-only.

[SYCL] Add and adjust tests for SYCL 2020 multi_ptr #1293

Merged
merged 2 commits into from
Nov 1, 2022
Merged

[SYCL] Add and adjust tests for SYCL 2020 multi_ptr #1293

merged 2 commits into from
Nov 1, 2022

Conversation

steffenlarsen
Copy link

This commit adjusts a number of existing using multi_ptr and adds new tests for new interfaces.

intel/llvm changes: intel/llvm#6893

@steffenlarsen
[SYCL] Add and adjust tests for SYCL 2020 multi_ptr
94ccfd9 
This commit adjusts a number of existing using multi_ptr and adds new
tests for new interfaces.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen steffenlarsen mentioned this pull request Sep 28, 2022
Merged
@steffenlarsen
Fix formatting
12025c7 
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen
Copy link
Author

/verify with intel/llvm#6893

steffenlarsen added a commit to intel/llvm that referenced this pull request Oct 13, 2022
@steffenlarsen
[SYCL] Implement SYCL 2020 multi_ptr (#6893)
8700b76 
This commit adds implementations of [SYCL 2020
`multi_ptr`](https://registry.khronos.org/SYCL/specs/sycl-2020/html/sycl-2020.html#sec:multiptr)
and `address_space_cast`. Likewise it adds the `legacy` decoration for
SYCL 1.2.1 style `multi_ptr` as deprecated.
To prevent breaking user code the legacy decoration is made the default.

Test-suite changes: intel/llvm-test-suite#1293

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen
Copy link
Author

@Pennycook & @againull - Friendly ping.

@Pennycook
Copy link

Sorry for the delay and thanks for the ping.

Can you say a little bit about why some of the changes use the legacy decoration and some of them use yes?

@steffenlarsen
Copy link
Author

Can you say a little bit about why some of the changes use the legacy decoration and some of them use yes?

Extensions with functionality that returns multi_ptr will return legacy until they have been changed. This is because the new multi_ptr implementation defaults the access::decorated template argument to legacy for backwards compatibility. So in principle, the cases here where legacy is used could stay unchanged, but I think it's better to be explicit, to make it simpler to transition to not having a default access::decorated value in the future.

As for the extensions, I think we need to consider how to transition these interfaces to use the non-legacy versions, then return to these and adjust accordingly.

@Pennycook
Copy link

Ah, I see. I'd missed that all the legacy usages were for return values. That makes sense, thanks for the explanation.

@steffenlarsen
Copy link
Author

@againull - Friendly ping.

againull
againull approved these changes Nov 1, 2022
View reviewed changes
Copy link

@againull againull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm so sorry.

@againull againull merged commit cb20604 into intel:intel Nov 1, 2022
myler pushed a commit to myler/llvm-test-suite that referenced this pull request Mar 22, 2023
@steffenlarsen @bb-sycl
[SYCL] Add and adjust tests for SYCL 2020 multi_ptr (intel#1293)
72ea5c0 
This commit adjusts a number of existing using multi_ptr and adds new
tests for new interfaces.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
@steffenlarsen
[SYCL] Add and adjust tests for SYCL 2020 multi_ptr (intel/llvm-test-…
c9b8d46 
…suite#1293)

This commit adjusts a number of existing using multi_ptr and adds new
tests for new interfaces.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Pennycook Pennycook Pennycook approved these changes

@againull againull againull approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@steffenlarsen @Pennycook @againull