Skip to content

[SYCL][Bindless][Doc][ABI-Break] Add const qualifiers to copies #14140

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
Merged
merged 18 commits into from
Jul 17, 2024
Merged

[SYCL][Bindless][Doc][ABI-Break] Add const qualifiers to copies #14140

merged 18 commits into from
Jul 17, 2024

Conversation

@Seanst98
Copy link
Contributor

@Seanst98 Seanst98 commented Jun 11, 2024

Add missing const qualifiers to Src parameter in ext_oneapi_copy functions and bindless spec.

This is an ABI-Breaking change not intended for merging until the ABI-Break window.

Corresponding UR PR: oneapi-src/unified-runtime#1743

@Seanst98 Seanst98 requested review from a team as code owners June 11, 2024 12:37
@Seanst98 Seanst98 mentioned this pull request Jun 11, 2024
Merged
@Seanst98
[SYCL][Bindless][Doc][ABI-Break] Add const qualifiers to copies
574f10d 
Add missing const qualifiers to Src parameter in ext_oneapi_copy functions and
bindless spec.

This is an ABI-Breaking change not intended for merging until the
ABI-Break window.
@Seanst98 Seanst98 force-pushed the sean/copy-const-qualifiers branch from 83ddea8 to 574f10d Compare June 11, 2024 12:41
uditagarwal97
uditagarwal97 approved these changes Jun 20, 2024
View reviewed changes
Copy link
Contributor

@uditagarwal97 uditagarwal97 left a comment

Choose a reason for hiding this comment

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

SYCL changes LGTM.
@Seanst98 since this PR is not intended to be merged till the ABI-breaking window, can we convert it to draft for now, to prevent accidental merges?

@steffenlarsen steffenlarsen added the abi-break change that's breaking abi and waiting for the next window to be able to merge label Jun 20, 2024
@Seanst98
Copy link
Contributor Author

Seanst98 commented Jul 11, 2024

Friendly ping @intel/dpcpp-nativecpu-pi-reviewers.

PietroGhg
PietroGhg reviewed Jul 11, 2024
View reviewed changes
PietroGhg
PietroGhg approved these changes Jul 11, 2024
View reviewed changes
Copy link
Contributor

@PietroGhg PietroGhg left a comment

Choose a reason for hiding this comment

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

Native CPU LGTM, thank you

kbenzie
kbenzie reviewed Jul 16, 2024
View reviewed changes
Copy link
Contributor

@kbenzie kbenzie left a comment

Choose a reason for hiding this comment

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

oneapi-src/unified-runtime#1743 has been merged, please update the UR repo/tag as suggested and pull in the latest sycl branch changes.

@Seanst98
Copy link
Contributor Author

Seanst98 commented Jul 17, 2024

@intel/llvm-gatekeepers can we merge this please? The CI is failing for one test which is unrelated to this PR and is being addressed in this PR: #14589

@steffenlarsen steffenlarsen mentioned this pull request Jul 17, 2024
Merged
@steffenlarsen
Copy link
Contributor

steffenlarsen commented Jul 17, 2024

As mentioned in #14555, I would like us to hold these a little yet until we either merge #14145 or get a green light from them to proceed with these bindless images changes. I apologize for the inconvenience.

@kbenzie
Copy link
Contributor

kbenzie commented Jul 17, 2024

As mentioned in #14555, I would like us to hold these a little yet until we either merge #14145 or get a green light from them to proceed with these bindless images changes. I apologize for the inconvenience.

So this actually pulls in additional UR changes that are needed for #14145 so am happy for this to merge first.

@steffenlarsen steffenlarsen merged commit fb561b9 into intel:sycl Jul 17, 2024
@Seanst98 Seanst98 deleted the sean/copy-const-qualifiers branch July 19, 2024 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@uditagarwal97 uditagarwal97 uditagarwal97 approved these changes

+5 more reviewers

@kbenzie kbenzie kbenzie approved these changes

@MartinWehking MartinWehking MartinWehking approved these changes

@isaacault isaacault isaacault approved these changes

@PietroGhg PietroGhg PietroGhg approved these changes

@DBDuncan DBDuncan DBDuncan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

abi-break change that's breaking abi and waiting for the next window to be able to merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@Seanst98 @steffenlarsen @kbenzie @MartinWehking @uditagarwal97 @isaacault @PietroGhg @DBDuncan