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.

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

Release SYCL_INTEL_enqueue_barrier extension document #1199

Merged
merged 1 commit into from
Mar 18, 2020

Conversation

mkinsner
Copy link

Signed-off-by: michael.kinsner michael.kinsner@intel.com

Signed-off-by: michael.kinsner <michael.kinsner@intel.com>
@mkinsner mkinsner requested review from bader and jbrodman February 26, 2020 13:47
Copy link
Contributor

@jbrodman jbrodman left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@Ruyk Ruyk left a comment

Choose a reason for hiding this comment

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

Thanks, that is a useful addition! Just a few comments

|`void barrier( const vector_class<event> &waitList )` | `event submit_barrier( const vector_class<event> &waitList )`
|========================================

The first variant of the barrier takes no parameters, and waits for all previously submitted commands to the queue to enter the `info::event_command_status::complete` state before any command later submitted to the same queue is allowed to execute. A second variant of the barrier accepts a list of events, with the behavior that no commands submitted to the same queue after barrier submission may execute until all events in the `waitList` have entered the `info::event_command_status::complete` state. Both variants are non-blocking from the host program perspective, in that they do not wait for the barrier conditions to have been met before returning.
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if the submit_barrier version that takes a vector_class<event> receives an empty list? does it act like the submit_barrier with no parameters, waiting for all submitted command groups to complete, or there is no wait at all? In that case, the event returned is immediately completed?

Copy link

Choose a reason for hiding this comment

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

This is described below in 4.8.X table:
If waitList is empty, then the barrier has no effect.
Yes, the event returned is immediately completed.

Copy link
Author

@mkinsner mkinsner Mar 6, 2020

Choose a reason for hiding this comment

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

I think the behavior is fully defined on this. @Ruyk If you disagree or want the material duplicated in this overview section, please comment.

|========================================
|*Member functions*|*Description*
|`event submit_barrier()` | Same effect as submitting a `handler::barrier()` within a command group to this `queue`. The returned event enters the `info::event_command_status::complete` state when all events that the barrier is dependent on (implicitly from all previously submitted commands to the same queue) have entered the `info::event_command_status::complete` state.
|`event submit_barrier( const vector_class<event> &waitList )` | Same effect as submitting a `handler:barrier( const vector_class<event> &waitList )` within a command group to this `queue`. The returned event enters the `info::event_command_status::complete` state when all events that the barrier is dependent on (explicitly from `waitList`) have entered the `info::event_command_status::complete` state.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can any of these methods throw any exception? e.g. should it throw an exception if the waitList is empty? is there any known condition where the barrier will fail to execute? If that is the case, is that a synchronous exception or asynchronous (via callback?)

Copy link

Choose a reason for hiding this comment

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

I don't have strong opinion. Any comments? @mkinsner

Copy link
Author

Choose a reason for hiding this comment

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

I don't think there should be an exception on empty waitList because there are good use cases for a dynamic list that we want to enable, and the list can happen to be empty.

I suppose that there could be error conditions if an event in waitList is invalid, but SYCL generally doesn't define these sorts of error conditions. So in short, I'm not aware of any condition that we should define which would throw an exception from this API, and therefore I'd choose to not say anything about them in this version of the extension.

@bader
Copy link
Contributor

bader commented Mar 11, 2020

@Ruyk, do you have any other comments or it's okay to merge?

@bader
Copy link
Contributor

bader commented Mar 13, 2020

@Ruyk, ping.

@bader bader merged commit 6cfd2cb into intel:sycl Mar 18, 2020
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Mar 20, 2020
* sycl: (1209 commits)
  [SYCL] Check exit status get_device_count_by_type
  [SYCL][Doc] Update sub-group extension docs (intel#1330)
  [SYCL][Doc] Add leader to GroupAlgorithms (intel#1297)
  [SYCL] Add SYCL headers search path to default compilation options (intel#1347)
  [SYCL][PI] Add interoperability with generic handles to device and program classes (intel#1244)
  Move SPIR devicelib to top level (intel#1276)
  [SYCL][Driver] Improve fat static library support (intel#1319)
  [SYCL] Remove image_api LIT (intel#1349)
  [SYCL] Fix headers location for check-sycl-deploy target
  [SYCL] Allow gcc asm statements in kernel code (intel#1341)
  [SYCL] Add Intel FPGA force_pow2_depth attribute (intel#1284)
  [SPIR-V][NFC] Fix for building llvm-spirv with -DLLVM_LINK_LLVM_DYLIB=ON (intel#1323)
  [SYCL][NFC] Fix execution graph dump (intel#1331)
  [SYCL][Doc] Release SYCL_INTEL_enqueue_barrier extension document (intel#1199)
  [SYCL][USM] Fix USM malloc_shared and free to handle zero byte (intel#1273)
  [SYCL] Fix undefined symbols in async_work_group_copy (intel#1243)
  [SYCL] Mark calls to barrier and work-item functions as convergent
  [SYCL][CUDA] Fix CUDA plug-in build with enabled assertions (intel#1325)
  [SYCL][Test] Add OpenCL requirement to test/ordered_queue/prop.cpp (intel#1335)
  [SYCL][CUDA] Improve CUDA backend documentation (intel#1293)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants