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][Doc] Update ExtendedAtomics documentation #1487

Merged
merged 8 commits into from
Apr 14, 2020
Merged

[SYCL][Doc] Update ExtendedAtomics documentation #1487

merged 8 commits into from
Apr 14, 2020

Conversation

Pennycook
Copy link
Contributor

Several major changes to the extension:

  • No longer replaces cl::sycl::atomic
  • Closer alignment with C++20 atomic_ref
  • Supports additional memory orders

Signed-off-by: John Pennycook john.pennycook@intel.com

@Pennycook
[SYCL][Doc] Update ExtendedAtomics documentation
655d35b 
Several major changes to the extension:
- No longer replaces cl::sycl::atomic
- Closer alignment with C++20 atomic_ref
- Supports additional memory orders

Signed-off-by: John Pennycook <john.pennycook@intel.com>
@Pennycook Pennycook added the spec extension All issues/PRs related to extensions specifications label Apr 7, 2020
@Pennycook Pennycook requested a review from AlexeySachkov April 7, 2020 16:41
@Pennycook Pennycook force-pushed the extended-atomics-update branch from 69c745e to 6ccd249 Compare April 9, 2020 19:05
@Pennycook
[SYCL][Doc] Add atomic_fence to ExtendedAtomics
5c9cd0c 
Signed-off-by: John Pennycook <john.pennycook@intel.com>
@Pennycook Pennycook force-pushed the extended-atomics-update branch from 6ccd249 to 5c9cd0c Compare April 9, 2020 19:53
@Pennycook
Add missing const and noexcept
93eaf12 
Signed-off-by: John Pennycook <john.pennycook@intel.com>
@Pennycook
Swap ptrdiff_t for difference_type
03ef0b4 
Signed-off-by: John Pennycook <john.pennycook@intel.com>
@Pennycook
Add missing constructor
449db89 
Clarify text slightly to distinguish between constructors.

Signed-off-by: John Pennycook <john.pennycook@intel.com>
@Pennycook
Clarify allowed values of address_space template
c56a6e2 
Signed-off-by: John Pennycook <john.pennycook@intel.com>
@bader bader requested a review from AlexeySachkov April 12, 2020 09:16
@Pennycook
Add const noexcept to integral specialization
6ef0418 
Signed-off-by: John Pennycook <john.pennycook@intel.com>
@Pennycook
Add const noexcept to table entries
8bdab1e 
Signed-off-by: John Pennycook <john.pennycook@intel.com>
@bader bader requested a review from AlexeySachkov April 14, 2020 06:21
AlexeySachkov
AlexeySachkov reviewed Apr 14, 2020
View reviewed changes
Copy link
Contributor

@AlexeySachkov AlexeySachkov left a comment

Choose a reason for hiding this comment

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

I don't have any further comments/suggestions, looks good to me.

sycl/doc/extensions/ExtendedAtomics/SYCL_INTEL_extended_atomics.asciidoc

==== To:
void atomic_fence(memory_order order):
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I right that the reason why atomic_fence is a free function and not a member of item or nd_item class (like barrier, for example) is that atomic_fence might be needed even in single-threaded application, just to ensure that instructions order will not be changed by compiler and/or HW implementation details - right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are two additional reasons: 1) atomic_thread_fence is a free function in standard C++; and 2) atomic operations with acquire or release semantics behave very similarly to fences, and it seemed weird to limit one to ND-range kernels but not the other.

@bader bader merged commit 1084685 into intel:sycl Apr 14, 2020
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Apr 15, 2020
Merge remote-tracking branch 'origin/sycl' into private/abatashe/sche…
160d890 
…duler_docs

* origin/sycl:
  [SYCL][PI][CUDA] Implements get_native interoperability (intel#1332)
  [SYCL] Fix check-sycl test suite on systems w/o OpenCL (intel#1503)
  [SYCL][Doc] Update ExtendedAtomics documentation (intel#1487)
  [SYCL][CUDA] Expose context extended deleters on PI API (intel#1483)
  [SYCL][NFC] Remove a dropped environment variable from a test (intel#1506)
  [SYCL] Add opencl-aot to sycl-toolchain target (intel#1504)
  [SYCL] Allow to run deploy LIT tests from particular directory
  [SYCL][CUDA] Fix LIT testing with CUDA devices (intel#1300)
  [SYCL] Remove operator name keywords (intel#1501)
  [Driver][SYCL] Consider .lo files as static archives (intel#1500)
  [SYCL-PTX] Update the compiler design to describe the CUDA target (intel#1408)
  [SYCL] Fix library build on Windows (intel#1499)
  [SYCL][NFC] Refactor lit.cfg.py (intel#1452)
  [SYCL] Fixed sub-buffer memory allocation update (intel#1486)
  [SYCL] Ensure proper definition of spirv builtins for SYCL (intel#1393)
  [SYCL][CUDA] LIT XFAIL/UNSUPPORTED (intel#1303)
  [SYCL][Doc] Function-type kernel attribute extension (intel#1494)
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Apr 15, 2020
Merge remote-tracking branch 'origin/sycl' into private/abatashe/basi…
07e1b93 
…c_abi_checks

* origin/sycl: (32 commits)
  [SYCL] Do not force LLVM_INCLUDE_TESTS variable (intel#1505)
  [SYCL][NFC] Align nd_item members with constructor initialization list (intel#1521)
  [SYCL] Move get_info_host implementation to header (intel#1514)
  [SYCL] Always use dynamic CRT for Unit tests (intel#1515)
  [SYCL][NFC] Temporarily disable sporadically failing test (intel#1526)
  [SYCL] Fix inline namespaces (intel#1525)
  [SYCL] Release notes for March'20 DPCPP implementation update (intel#1511)
  [SYCL][PI][CUDA] Implements get_native interoperability (intel#1332)
  [SYCL] Fix check-sycl test suite on systems w/o OpenCL (intel#1503)
  [SYCL][Doc] Update ExtendedAtomics documentation (intel#1487)
  [SYCL][CUDA] Expose context extended deleters on PI API (intel#1483)
  [SYCL][NFC] Remove a dropped environment variable from a test (intel#1506)
  [SYCL] Add opencl-aot to sycl-toolchain target (intel#1504)
  [SYCL] Allow to run deploy LIT tests from particular directory
  [SYCL][CUDA] Fix LIT testing with CUDA devices (intel#1300)
  [SYCL] Remove operator name keywords (intel#1501)
  [Driver][SYCL] Consider .lo files as static archives (intel#1500)
  [SYCL-PTX] Update the compiler design to describe the CUDA target (intel#1408)
  [SYCL] Fix library build on Windows (intel#1499)
  [SYCL][NFC] Refactor lit.cfg.py (intel#1452)
  ...
@Pennycook Pennycook deleted the extended-atomics-update branch July 10, 2020 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexeySachkov AlexeySachkov AlexeySachkov left review comments

@mkinsner mkinsner mkinsner approved these changes

@jbrodman jbrodman Awaiting requested review from jbrodman

Assignees
No one assigned
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.
4 participants
@Pennycook @AlexeySachkov @mkinsner @bader