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.

Sign up for GitHub

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

Jump to bottom

Undeprecate legacy multi_ptr #604

Merged
merged 3 commits into from
Oct 24, 2024
Merged

Undeprecate legacy multi_ptr #604

merged 3 commits into from
Oct 24, 2024

Conversation

steffenlarsen
Copy link
Contributor

The legacy decoration for multi_ptr has been made the default template argument of the decoration in multi_ptr to bridge the gap between SYCL 1.2.1 and SYCL 2020 multi_ptr interfaces. However, the legacy decoration and the specialization of multi_ptr with the decoration is deprecated with SYCL 2020. This is confusing to users as they may easily run into deprecation warnings simply due to not specifying the decoration. This commit proposes that we remove the deprecation of legacy in SYCL 2020.

@steffenlarsen
Undeprecate legacy multi_ptr
b393ab6 
The legacy decoration for multi_ptr has been made the default template
argument of the decoration in multi_ptr to bridge the gap between
SYCL 1.2.1 and SYCL 2020 multi_ptr interfaces. However, the legacy
decoration and the specialization of multi_ptr with the decoration is
deprecated with SYCL 2020. This is confusing to users as they may easily
run into deprecation warnings simply due to not specifying the
decoration. This commit proposes that we remove the deprecation of
legacy in SYCL 2020.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@sycl-issue-bot sycl-issue-bot bot mentioned this pull request Aug 16, 2024
Closed
@steffenlarsen
Copy link
Contributor Author

Note that async_work_group_copy using legacy multi_ptr is still deprecated. We can undeprecate them, but their signatures will be slightly different from the yes-no deprecated ones.

@tomdeakin tomdeakin added the Agenda To be discussed during a SYCL committee meeting label Aug 19, 2024
@VerenaBeckham
Copy link
Contributor

VerenaBeckham commented Aug 22, 2024 •
edited
Loading

How about the statement in programming_interface.adoc:

Note that using [code]#global_ptr#, [code]#local_ptr#, [code]#constant_ptr# or
[code]#private_ptr# without specifying the decoration is deprecated.
The default argument is provided for compatibility with 1.2.1.

Should that also be removed as part of this PR?

@tomdeakin tomdeakin removed the Agenda To be discussed during a SYCL committee meeting label Aug 28, 2024
@steffenlarsen
Remove alias deprecation note
f495a77 
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen
Copy link
Contributor Author

How about the statement in programming_interface.adoc:

Note that using [code]#global_ptr#, [code]#local_ptr#, [code]#constant_ptr# or
[code]#private_ptr# without specifying the decoration is deprecated.
The default argument is provided for compatibility with 1.2.1.

Should that also be removed as part of this PR?

Good catch! It has been removed.

VerenaBeckham
VerenaBeckham approved these changes Aug 29, 2024
View reviewed changes
Copy link
Contributor

@VerenaBeckham VerenaBeckham left a comment

Choose a reason for hiding this comment

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

I can't comment on whether this change is the right thing to do, but the code LGTM.

@AerialMantis
Copy link
Collaborator

I think we also need to update the change log, that currently states that the legacy option for multi_ptr is deprecated.

  • legacy 1.2.1 interface (deprecated).

AerialMantis
AerialMantis approved these changes Sep 10, 2024
View reviewed changes
Copy link
Collaborator

@AerialMantis AerialMantis left a comment

Choose a reason for hiding this comment

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

I think we might need to update the change log as well, but other than that this change LGTM.

@steffenlarsen
Remove deprecation entry in changelog
8373205 
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen steffenlarsen added the Agenda To be discussed during a SYCL committee meeting label Oct 22, 2024
@gmlueck
Copy link
Contributor

gmlueck commented Oct 24, 2024

WG approved.

@gmlueck gmlueck merged commit f652da8 into KhronosGroup:main Oct 24, 2024
2 checks passed
steffenlarsen added a commit to steffenlarsen/llvm that referenced this pull request Oct 28, 2024
@steffenlarsen
[SYCL] Remove deprecation of legacy multi_ptr
4cc1993 
With KhronosGroup/SYCL-Docs#604 the SYCL
specification no longer deprecates legacy multi_ptr. This commit removes
the corresponding deprecation warnings from the implementation of the
specializations and the enum value.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen steffenlarsen mentioned this pull request Oct 28, 2024
Open
gmlueck added a commit to gmlueck/SYCL-Docs that referenced this pull request Oct 31, 2024
@gmlueck
Undeprecate legacy multi_ptr
e8aad21 
Cherry pick KhronosGroup#604 from main
(cherry picked from commit f652da8)
@gmlueck gmlueck mentioned this pull request Oct 31, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nliber nliber nliber approved these changes

@keryell keryell keryell approved these changes

@gmlueck gmlueck gmlueck approved these changes

@AerialMantis AerialMantis AerialMantis approved these changes

@VerenaBeckham VerenaBeckham VerenaBeckham approved these changes

Assignees
No one assigned
Labels
Agenda To be discussed during a SYCL committee meeting
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@steffenlarsen @VerenaBeckham @AerialMantis @gmlueck @nliber @keryell @tomdeakin