Skip to content

[SYCL] Add prototype of group algorithms #1236

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 19 commits into from
Mar 17, 2020
Merged

[SYCL] Add prototype of group algorithms #1236

merged 19 commits into from
Mar 17, 2020

Conversation

Pennycook
Copy link
Contributor

Exposes group collectives:

  • all_of
  • any_of
  • none_of
  • reduce
  • exclusive_scan
  • inclusive_scan

This prototype does not support the host device.

Co-Authored-By: Roland Schulz [email protected]
Co-Authored-By: Alexey Sachkov [email protected]

Signed-off-by: John Pennycook [email protected]

@Pennycook Pennycook added the spec extension All issues/PRs related to extensions specifications label Mar 3, 2020
@Pennycook Pennycook requested review from bader and AlexeySachkov March 3, 2020 17:28
@Pennycook
Copy link
Contributor Author

@bader There was a formatting error in sycl.hpp, but on a line I didn't change. I fixed it to make the check pass, but wanted to point this out in case it's not the expected behavior of the clang-format-check script.

@bader
Copy link
Contributor

bader commented Mar 3, 2020

@bader There was a formatting error in sycl.hpp, but on a line I didn't change. I fixed it to make the check pass, but wanted to point this out in case it's not the expected behavior of the clang-format-check script.

@Pennycook, thanks for taking care. Are you talking about includes order? If so, I think the check is applied for all includes not separated by an empty line.

@Pennycook
Copy link
Contributor Author

@Pennycook, thanks for taking care. Are you talking about includes order? If so, I think the check is applied for all includes not separated by an empty line.

Right, it was the include order. I understand why clang-format made the change, but it seemed to be complaining about an include that I hadn't added! Looking at the final diff everything looks fine, though, so please ignore this. Probably the order got fixed in a previous commit, and I was accidentally undoing the change.

@Pennycook
Copy link
Contributor Author

Pennycook commented Mar 4, 2020 •
edited
Loading

I think the test that's failing is unrelated to the changes here. I'll try again after #1245 is merged.

@romanovvlad romanovvlad requested a review from v-klochkov March 4, 2020 16:23
bader
bader reviewed Mar 11, 2020
View reviewed changes
Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

LGTM, just a few minor comments.

AlexeySachkov
AlexeySachkov reviewed Mar 11, 2020
View reviewed changes
bader
bader previously approved these changes Mar 11, 2020
View reviewed changes
Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

👍

@Pennycook
Copy link
Contributor Author

Added new algorithm from #1297.

@Pennycook Pennycook requested a review from bader March 12, 2020 18:04
AlexeySachkov
AlexeySachkov reviewed Mar 12, 2020
View reviewed changes
bader
bader requested changes Mar 13, 2020
View reviewed changes
Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

@Pennycook, please, fix build issues.

bader
bader previously approved these changes Mar 13, 2020
View reviewed changes
@Pennycook
[SYCL] Refactor sub-group code for reuse
9ada7f4 
Preparation to re-use code for work-group collectives:
- Move calc to functional.hpp
- Make SPIR-V scope an explicit argument to calc
- Add C++ helper for __spirv_GroupBroadcast

Signed-off-by: John Pennycook <[email protected]>
@Pennycook
[SYCL] Add static members to group class
a61ecd0 
Simplifies definition of library functions by providing:
- id_type
- range_type
- linear_id_type
- dimensions

Signed-off-by: John Pennycook <[email protected]>
@Pennycook
[SYCL] Add prototype of group algorithms
cea1028 
Exposes group collectives:
- all_of
- any_of
- none_of
- reduce
- exclusive_scan
- inclusive_scan

This prototype does not support the host device.

Co-Authored-By: Roland Schulz <[email protected]>
Co-Authored-By: Alexey Sachkov <[email protected]>

Signed-off-by: John Pennycook <[email protected]>
@Pennycook
Fix clang-format errors in group algorithms
58ac82d 
Signed-off-by: John Pennycook <[email protected]>
@Pennycook
Fix more clang-format errors in group algorithms
546101e 
Signed-off-by: John Pennycook <[email protected]>
@Pennycook
[SYCL] Convert typedefs to using syntax
29cf3f0 
Signed-off-by: John Pennycook <[email protected]>
@Pennycook
[SYCL] Remove copyright header from tests
3100d90 
Signed-off-by: John Pennycook <[email protected]>
@Pennycook
[SYCL] Capitalize template argument
cf1c593 
dimensions => Dimensions

Signed-off-by: John Pennycook <[email protected]>
Pennycook added 11 commits March 13, 2020 12:39
@Pennycook
[SYCL] Add comment to clarify broadcast overloads
3ffd441 
Signed-off-by: John Pennycook <[email protected]>
@Pennycook
[SYCL] Capitalize template argument
372ba33 
dimensions => Dimensions

Signed-off-by: John Pennycook <[email protected]>
@Pennycook
[SYCL] Add comment to #endif
8704202 
Signed-off-by: John Pennycook <[email protected]>
@Pennycook
[SYCL] Invert #ifdef logic for group algorithms
febf217 
Now enabled by default, disabled by:
__DISABLE_SYCL_INTEL_GROUP_ALGORITHMS__.

Signed-off-by: John Pennycook <[email protected]>
@Pennycook
[SYCL] Add prototype of leader algorithm
199ff41 
Signed-off-by: John Pennycook <[email protected]>
@Pennycook
[SYCL] Add PI_INVALID_DEVICE to runtime_error
a7d6504 
Signed-off-by: John Pennycook <[email protected]>
@Pennycook
[SYCL] Fix compilation with C++11
065f2b3 
- Remove generic lambdas
- Guard usage of transparent functors

Signed-off-by: John Pennycook <[email protected]>
@Pennycook
[SYCL] Rename function parameter id => linear_id
638fded 
Signed-off-by: John Pennycook <[email protected]>
@Pennycook
[SYCL] Make template specialization inline
7927e7e 
Avoids multiple definition errors.

Signed-off-by: John Pennycook <[email protected]>
@Pennycook
[SYCL] Do not rely on inline namespace
35b9c8a 
Signed-off-by: John Pennycook <[email protected]>
@Pennycook
[SYCL] Fix SPIR-V broadcast index
760761f 
uint32_t => size_t

Signed-off-by: John Pennycook <[email protected]>
@Pennycook Pennycook force-pushed the group-algorithms-prototype branch from a61240b to 760761f Compare March 13, 2020 19:41
@Pennycook
Copy link
Contributor Author

I think #1026 accidentally undid one of the changes from #1152, and that's what is causing the latest failures. I've rebased and re-applied the change from uint32_t to size_t.

@bader -- Looks like you were right, I should have included a regression test!

@bader bader merged commit 8bfa107 into intel:sycl Mar 17, 2020
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Mar 17, 2020
Merge remote-tracking branch 'origin/sycl' into private/abatashe/imag…
bc38516 
…e_api_test

* origin/sycl: (1188 commits)
  [SYCL][CUDA] Improve CUDA backend documentation (intel#1293)
  [SYCL] Emit textual IR when -S -fsycl-device-only is used (intel#1314)
  [SYCL] Add prototype of group algorithms (intel#1236)
  [SYCL] XFAIL test on windows to unblock pulldown
  Allow Intel Loop Controls only with SPV_INTEL_fpga_loop_controls
  Apply suggested assert msg change
  Implement SPV_INTEL_io_pipes extension
  [SYCL] Fix dependencies for SYCLLowerIR (intel#1321)
  [CI] Allow builds without pre-downloaded OpenCL in configure.py (intel#1317)
  [SYCL] Move SYCL headers from standard clang location (intel#1308)
  [mlir] Add support for generating dialect declarations via tablegen.
  Be more strict when checking existence of foo
  [CodeGenPrepare] Freeze condition when transforming select to br
  [ORC] Remove an undefined static method from LLJIT.
  [JITLink][AArch64] Fix incorrect capitalization in a testcase name.
  [ORC] Print symbol flags and materializer name in ExecutionSession::dump.
  [JITLink][MachO] Re-apply b64afad, MachO linker-private support, with fixes.
  Basic Block Sections Support.
  Test commit.
  [SYCL][Doc] Deploy documentation for PI (intel#1318)
  ...
@Pennycook Pennycook deleted the group-algorithms-prototype branch March 19, 2020 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbrodman jbrodman jbrodman left review comments

@AlexeySachkov AlexeySachkov AlexeySachkov left review comments

@bader bader bader approved these changes

@v-klochkov v-klochkov v-klochkov approved these changes

Assignees

@v-klochkov v-klochkov

Labels
spec extension All issues/PRs related to extensions specifications
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Pennycook @bader @jbrodman @AlexeySachkov @v-klochkov