Skip to content

[SYCL][CUDA][HIP][libclc] Rename libclc NonUniform binops to KHR extensions #6279

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 3 commits into from
Jun 9, 2022
Merged

[SYCL][CUDA][HIP][libclc] Rename libclc NonUniform binops to KHR extensions #6279

merged 3 commits into from
Jun 9, 2022

Conversation

steffenlarsen
Copy link
Contributor

#5705 changed the use of NonUniform group binary operators to the KHR extension operators. These changes rename the implementation of these in libclc for both amdgcn and PTX to the new naming.

@steffenlarsen
[SYCL][CUDA][HIP][libclc] Rename libclc NonUniform binops to KHR exte…
261fde4 
…nsions

intel#5705 changed the use of NonUniform
group binary operators to the KHR extension operators. These changes
rename the implementation of these in libclc for both amdgcn and PTX to
the new naming.

Signed-off-by: Larsen, Steffen <[email protected]>
@steffenlarsen steffenlarsen requested a review from JackAKirk June 9, 2022 13:20
@steffenlarsen steffenlarsen requested a review from a team as a code owner June 9, 2022 13:20
@JackAKirk
Copy link
Contributor

LGTM I've just noticed that in both files __CLC_SUBGROUP_COLLECTIVE is using IMul/FMul rather than IMulKHR/FMulKHR. Perhaps it would be good to change this here too?

smaslov-intel
smaslov-intel previously approved these changes Jun 9, 2022
View reviewed changes
JackAKirk
JackAKirk previously approved these changes Jun 9, 2022
View reviewed changes
@steffenlarsen steffenlarsen mentioned this pull request Jun 9, 2022
Merged
@steffenlarsen
Remove old comment
893b592 
Signed-off-by: Larsen, Steffen <[email protected]>
@steffenlarsen steffenlarsen dismissed stale reviews from JackAKirk and smaslov-intel via 893b592 June 9, 2022 14:20
@steffenlarsen
Copy link
Contributor Author

LGTM I've just noticed that in both files __CLC_SUBGROUP_COLLECTIVE is using IMul/FMul rather than IMulKHR/FMulKHR. Perhaps it would be good to change this here too?

I am not actually sure why this distinction is here, as the logical operators use the same name for subgroup collectives. I don't mind changing it, though I wonder if there is a reason for the different approach.

@steffenlarsen
Rename CLC subgroup collectives
429ee0a 
Signed-off-by: Larsen, Steffen <[email protected]>
@pvchupin pvchupin requested a review from smaslov-intel June 9, 2022 16:17
@JackAKirk
Copy link
Contributor

LGTM I've just noticed that in both files __CLC_SUBGROUP_COLLECTIVE is using IMul/FMul rather than IMulKHR/FMulKHR. Perhaps it would be good to change this here too?

I am not actually sure why this distinction is here, as the logical operators use the same name for subgroup collectives. I don't mind changing it, though I wonder if there is a reason for the different approach.

Yes I don't know either, I don't see a reason for it being different.

@pvchupin pvchupin merged commit 10c19ae into intel:sycl Jun 9, 2022
@steffenlarsen steffenlarsen deleted the steffen/libclc_rename_collectives branch December 6, 2023 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bader bader bader left review comments

@smaslov-intel smaslov-intel smaslov-intel approved these changes

@JackAKirk JackAKirk JackAKirk left review comments

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

Successfully merging this pull request may close these issues.
5 participants
@steffenlarsen @JackAKirk @bader @smaslov-intel @pvchupin