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

[SYCL] SYCL 2020 sub_group algorithms #3786

Merged
merged 31 commits into from
Jun 2, 2021
Merged

[SYCL] SYCL 2020 sub_group algorithms #3786

merged 31 commits into from
Jun 2, 2021

Conversation

cperkinsintel
Copy link
Contributor

@cperkinsintel cperkinsintel commented May 20, 2021 •
edited
Loading

Getting the functionality of the ONEAPI subgroup algorithms into the SYCL namespace, along with API updates, new names, tests, deprecation warnings.

This is based on an earlier version of @alexbatashev PR #3765 which as of this writing is not yet merged. I'll need to update.

@alexbatashev - I hope we have not stepped on each other too much with these two PRs. Let me know if you spot anything.

Alexander Batashev and others added 12 commits May 17, 2021 09:34
[SYCL] Implement SYCL2020 sub_group class
b8ea144
Revert some changes to avoid ABI breakage
2d386e9
fix build
85c3c55
@cperkinsintel
reduce => reduce_over_group. working. needs cleanup and deprecation w…
5f4db83 
…arnings. Also, ONEAPI::reduction() should be converted to use SYCL2020 reduce.

Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
clean up
9a3d034 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
deprecation of ONEAPI::reduce
02a5363 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
all_of_group, any_of_group, none_of_group
96b3e4a 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
is_group_v
fef5c73 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
tests
94a692f 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
shift_group_left/right
e4c17f7 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
select_from_group permute_group_by_xor
55708e4 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
exclusive/inclusive scan and broadcast. Tests and cleanup.
6d68ce5 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
clang-format has problem.
bf0494d 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
clang-formattery
e65cf34 
Signed-off-by: Chris Perkins <[email protected]>
vladimirlaz
vladimirlaz reviewed May 20, 2021
View reviewed changes
Copy link
Contributor

@vladimirlaz vladimirlaz left a comment

Choose a reason for hiding this comment

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

Some minor comments.

sycl/test/on-device/sub_group_algorithms/shift_left_right.hpp Outdated Show resolved Hide resolved
sycl/test/on-device/sub_group_algorithms/shift_left_right.cpp Outdated Show resolved Hide resolved
sycl/test/on-device/sub_group_algorithms/all_of.cpp Outdated Show resolved Hide resolved
sycl/test/on-device/sub_group_algorithms/all_of.cpp Outdated Show resolved Hide resolved
sycl/test/on-device/sub_group_algorithms/any_of.cpp Outdated Show resolved Hide resolved
sycl/test/on-device/sub_group_algorithms/exclusive_scan.cpp Outdated Show resolved Hide resolved
sycl/test/on-device/sub_group_algorithms/group_broadcast.cpp Outdated Show resolved Hide resolved
sycl/test/on-device/sub_group_algorithms/support.h Outdated Show resolved Hide resolved
Pennycook
Pennycook requested changes May 20, 2021
View reviewed changes
Copy link
Contributor

@Pennycook Pennycook left a comment

Choose a reason for hiding this comment

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

There are a few things that I think we should definitely remove, because they're not in SYCL 2020. There are a few others that I'm not sure about, because I'm unsure of whether they're mistakes in the SYCL 2020 specification or not.

@pvchupin: If this PR exposes an interface that is later removed because of a SYCL specification clarification, would we have to go through regular deprecation procedures?

sycl/include/CL/sycl/ONEAPI/sub_group.hpp Outdated Show resolved Hide resolved
sycl/include/CL/sycl/sub_group_algorithm.hpp Outdated Show resolved Hide resolved
sycl/include/CL/sycl/sub_group_algorithm.hpp Outdated Show resolved Hide resolved
sycl/include/CL/sycl/sub_group_algorithm.hpp Outdated Show resolved Hide resolved
sycl/include/CL/sycl/sub_group_algorithm.hpp Outdated Show resolved Hide resolved
sycl/include/CL/sycl/sub_group_algorithm.hpp Outdated Show resolved Hide resolved
sycl/include/CL/sycl/sub_group_algorithm.hpp Outdated Show resolved Hide resolved
sycl/include/CL/sycl/sub_group_algorithm.hpp Outdated Show resolved Hide resolved
sycl/include/CL/sycl/sub_group_algorithm.hpp Outdated Show resolved Hide resolved
sycl/include/CL/sycl/sub_group_algorithm.hpp Outdated Show resolved Hide resolved
@cperkinsintel
reviewer feedback. checkpoint.
779228b 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
switch to known_identity. Also had to move known_identity.
0ed9117 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
checkpoint
84665fb 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
signature changes
dbf12b9 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
merge of latest. checkpoint
ff90016
@cperkinsintel
rename sub_group_algorithm.hpp
1a1f3d5 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
reviewer feedback. Cleaving close to SYCL2020 - some specializations …
37ffa16 
…moved back to ONEAPI namespace. Rename 'sub-group-algorithm' to 'group-algorithm' where appropriate. Adjusted signatures as requested.

Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
comments. clang-format part 1
2b2758c 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
clang-format part 2
1dbb5c5 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
clang-format part 3. Not a good use of my time, need to investigate w…
475080d 
…hy git clang-format is not working correctly.

Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
Merge branch 'intel-sycl' into cperkins-sycl2020_sub_group-algorhythms
ed70d72
@cperkinsintel cperkinsintel marked this pull request as ready for review May 28, 2021 22:07
@cperkinsintel cperkinsintel requested review from AlexeySachkov and a team as code owners May 28, 2021 22:07
@cperkinsintel
Copy link
Contributor Author

cperkinsintel commented May 28, 2021 •
edited by vladimirlaz
Loading

@vladimirlaz - can you help me understand this Jenkins/Precommit failure? I'm attempting to re-run the Parallel Stage. Not sure if that is the correct course of action.

After re-running, it passes.

Not sure how to get GitHub to update its status.

@cperkinsintel cperkinsintel requested a review from Pennycook May 28, 2021 22:10
@vladimirlaz
Copy link
Contributor

@vladimirlaz - can you help me understand this Jenkins/Precommit failure? ProtextIP is unable to make a connection? I'm attempting to re-run the Parallel Stage. Not sure if that is the correct course of action.

After re-running, it passes: http://icl-jenkins.sc.intel.com:8080/blue/organizations/jenkins/SYCL_CI%2Fintel%2FProtexIP/detail/ProtexIP/7222/pipeline/

Not sure how to get GitHub to update its status.

@vladimirlaz - can you help me understand this Jenkins/Precommit failure? I'm attempting to re-run the Parallel Stage. Not sure if that is the correct course of action.

After re-running, it passes.

Not sure how to get GitHub to update its status.

The overall status is not updated if internal job is restarted. There was scheduled downtime for the service. The job can be treated as passed if the internal job passed after restart.

@cperkinsintel
removing GroupNonUniformElect - vestigial branch and merge artifact. …
88a36cd 
…Not called. Originally removed by A. Batashev.

Signed-off-by: Chris Perkins <[email protected]>
vladimirlaz
vladimirlaz suggested changes Jun 1, 2021
View reviewed changes
sycl/include/CL/__spirv/spirv_ops.hpp Outdated Show resolved Hide resolved
sycl/include/CL/sycl/group_algorithm.hpp Outdated Show resolved Hide resolved
sycl/include/CL/sycl/group_algorithm.hpp Show resolved Hide resolved
sycl/include/CL/sycl/known_identity.hpp Show resolved Hide resolved
sycl/include/CL/sycl/known_identity.hpp Show resolved Hide resolved
sycl/include/CL/sycl/known_identity.hpp Show resolved Hide resolved
sycl/test/on-device/group_algorithms_sycl2020/all_of.cpp Outdated Show resolved Hide resolved
Pennycook
Pennycook requested changes Jun 1, 2021
View reviewed changes
Copy link
Contributor

@Pennycook Pennycook left a comment

Choose a reason for hiding this comment

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

joint_any_of, joint_all_of and joint_none_of in SYCL 2020 don't have any requirements on the input types, because applying Predicate must convert them to bool: https://www.khronos.org/registry/SYCL/specs/sycl-2020/html/sycl-2020.html#_any_of_all_of_and_none_of

Sorry I missed these in my initial review. I didn't see anything else, so I'll approve after these three changes.

sycl/include/CL/sycl/group_algorithm.hpp Show resolved Hide resolved
sycl/include/CL/sycl/group_algorithm.hpp Show resolved Hide resolved
sycl/include/CL/sycl/group_algorithm.hpp Show resolved Hide resolved
Pennycook
Pennycook previously approved these changes Jun 1, 2021
View reviewed changes
@cperkinsintel
reviewer feedback
eb214a1 
Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
clang-format?
53ef04b 
Signed-off-by: Chris Perkins <[email protected]>
vladimirlaz
vladimirlaz previously approved these changes Jun 2, 2021
View reviewed changes
@bader bader merged commit e8caf6c into intel:sycl Jun 2, 2021
@Pennycook Pennycook mentioned this pull request Jun 3, 2021
Merged
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Jun 4, 2021
Merge branch 'sycl' into group_barrier
614a0c4 
* sycl: (320 commits)
  [SYCL] Silence a "local variable is initialized but not referenced" warning; NFC (intel#3870)
  [SYCL] Improve SYCL_DEVICE_ALLOWLIST (intel#3826)
  [SPIR-V] Change return value of mapType function (intel#3871)
  [SYCL] Fix post-commit failure in handler.hpp from unused-parameters. (intel#3874)
  [Driver][SYCL] Do not imply defaultlib msvcrt for Linux based driver on Windows (intel#3827)
  [SYCL] Unique stable name rebase (intel#3835)
  [SYCL] Align behavior of empty command groups with SYCL2020 (intel#3822)
  [SYCL][ESIMD] Make typenames and constants consistent with SYCL API style. (intel#3850)
  [SYCL] Allow __failed_assertion to support libstdc++-11 (intel#3774)
  [SYCL] Refactor stream class handing implementation (intel#3646)
  [SYCL] Fix syntax error introduced in intel#3401 (intel#3861)
  [SYCL] SYCL 2020 sub_group algorithms (intel#3786)
  [Buildbot][NFC] Add option to use LLD as linker (intel#3866)
  Revert "Emit correct location lists with basic block sections."
  [SPIRITTAnnotations] Fix debug info for ITT calls. (intel#3829)
  [SYCL][Doc] Fix build of Sphinx docs (intel#3863)
  [SYCL][FPGA][NFC] Tidy up intel_fpga_reg codegen test (intel#3810)
  [CODEOWNERS] Fix SPIRITTAnnnotations tests ownership (intel#3859)
  [SYCL][ESIMD] Host-compile simd.cpp test, fix errors & warnings. (intel#3846)
  [SYCL] Store pointers to memory allocations instead of iterators (intel#3860)
  ...
@masterleinad masterleinad mentioned this pull request Jul 28, 2021
Merged
@Pennycook Pennycook mentioned this pull request Oct 7, 2021
Closed
iclsrc pushed a commit that referenced this pull request Dec 8, 2023
@vpykhtin
[AMDGPU] Fix GCNUpwardRPTracker: max register pressure on defs. (#74422)
901c5be 
Treat a defined register as fully live "at" the instruction and update maximum pressure accordingly. Fixes #3786.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pennycook Pennycook Pennycook left review comments

@alexbatashev alexbatashev alexbatashev left review comments

@bader bader bader left review comments

@vladimirlaz vladimirlaz vladimirlaz left review comments

@AlexeySachkov AlexeySachkov Awaiting requested review from AlexeySachkov

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
@cperkinsintel @vladimirlaz @bader @alexbatashev @Pennycook