Skip to content

[SYCL] Use uniform group instructions in headers #5705

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
vladimirlaz merged 4 commits into from
Mar 22, 2022
Merged

[SYCL] Use uniform group instructions in headers #5705

vladimirlaz merged 4 commits into from
Mar 22, 2022

Conversation

@aobolensk
Copy link
Contributor

@aobolensk aobolensk commented Mar 2, 2022

No description provided.

@KornevNikita
Copy link
Contributor

KornevNikita commented Mar 2, 2022

There are changes in extension vendor & operations name. @MrSidims will we promote it as INTEL extension now and then change it to KHR?

@MrSidims
Copy link
Contributor

MrSidims commented Mar 2, 2022

@aobolensk please update the instructions' names to the names used here: KhronosGroup/SPIRV-Registry#138

KornevNikita
KornevNikita reviewed Mar 3, 2022
View reviewed changes
llvm-spirv/include/LLVMSPIRVExtensions.inc Outdated
Copy link
Contributor

@KornevNikita KornevNikita Mar 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I don't sure if we need it here, until the extension is ready and transfered to intel/llvm repository: KhronosGroup/SPIRV-LLVM-Translator#1253. The same about Clang.cpp. @MrSidims WDYT?

Copy link
Contributor

@MrSidims MrSidims Mar 3, 2022

Choose a reason for hiding this comment

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

@aobolensk please remove this change from the translator, let the translator's work be merged and pulled to intel/llvm first.

MrSidims
MrSidims requested changes Mar 3, 2022
View reviewed changes
llvm-spirv/include/LLVMSPIRVExtensions.inc Outdated
Copy link
Contributor

@MrSidims MrSidims Mar 3, 2022

Choose a reason for hiding this comment

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

@aobolensk please remove this change from the translator, let the translator's work be merged and pulled to intel/llvm first.

clang/lib/Driver/ToolChains/Clang.cpp Outdated
Copy link
Contributor

@MrSidims MrSidims Mar 3, 2022

Choose a reason for hiding this comment

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

Let's enable the extension on any target but FPGA H/W aka move this code to line 9154.

@KornevNikita
Copy link
Contributor

KornevNikita commented Mar 18, 2022

@aobolensk KhronosGroup/SPIRV-LLVM-Translator#1253 is in intel/llvm now

@aobolensk aobolensk force-pushed the uniform_group_instructions branch from 5a4bed5 to 121db8b Compare March 18, 2022 15:25
@aobolensk
Copy link
Contributor Author

aobolensk commented Mar 18, 2022

@aobolensk KhronosGroup/SPIRV-LLVM-Translator#1253 is in intel/llvm now

Thanks! Rebased this on top of merged change

@aobolensk aobolensk marked this pull request as ready for review March 21, 2022 07:20
@aobolensk aobolensk requested review from a team as code owners March 21, 2022 07:20
alexbatashev
alexbatashev approved these changes Mar 21, 2022
View reviewed changes
Copy link
Contributor

@alexbatashev alexbatashev left a comment

Choose a reason for hiding this comment

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

Runtime LGTM

Fznamznon
Fznamznon reviewed Mar 21, 2022
View reviewed changes
Fznamznon
Fznamznon approved these changes Mar 21, 2022
View reviewed changes
Copy link
Contributor

@Fznamznon Fznamznon left a comment

Choose a reason for hiding this comment

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

FE changes look ok.

smanna12
smanna12 approved these changes Mar 21, 2022
View reviewed changes
Copy link
Contributor

@smanna12 smanna12 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

FE changes look good to me!

hchilama
hchilama approved these changes Mar 21, 2022
View reviewed changes
Copy link
Contributor

@hchilama hchilama left a comment

Choose a reason for hiding this comment

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

Driver files look ok.

@vladimirlaz vladimirlaz merged commit 9b84dd8 into intel:sycl Mar 22, 2022
@aobolensk aobolensk mentioned this pull request Mar 22, 2022
Merged
steffenlarsen added a commit to steffenlarsen/llvm that referenced this pull request Jun 9, 2022
@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 <steffen.larsen@intel.com>
@steffenlarsen steffenlarsen mentioned this pull request Jun 9, 2022
Merged
pvchupin pushed a commit that referenced this pull request Jun 9, 2022
@steffenlarsen
[SYCL][CUDA][HIP][libclc] Rename libclc NonUniform binops to KHR exte…
10c19ae 
…nsions (#6279)

#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 <steffen.larsen@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Fznamznon Fznamznon Fznamznon approved these changes

@MrSidims MrSidims MrSidims approved these changes

@hchilama hchilama hchilama approved these changes

@KornevNikita KornevNikita Awaiting requested review from KornevNikita

+3 more reviewers

@alexbatashev alexbatashev alexbatashev approved these changes

@vladimirlaz vladimirlaz vladimirlaz approved these changes

@smanna12 smanna12 smanna12 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@aobolensk @KornevNikita @MrSidims @alexbatashev @Fznamznon @vladimirlaz @smanna12 @hchilama