Skip to content

[SYCL][L0] Fix piEnqueueEventsWaitWithBarrier for L0 #6359

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 12 commits into from
Jul 11, 2022
Merged

[SYCL][L0] Fix piEnqueueEventsWaitWithBarrier for L0 #6359

merged 12 commits into from
Jul 11, 2022

Conversation

steffenlarsen
Copy link
Contributor

When calling piEnqueueEventsWaitWithBarrier while using the level zero backend, the barrier is only inserted into an available command list. However, this leaves other command lists unaffected.

This commit fixes this unexpected behavior by making later command-lists insert a barrier on events from piEnqueueEventsWaitWithBarrier until they finish.

The resulting event will be either the result of a single barrier, if events were passed to piEnqueueEventsWaitWithBarrier, or a wait on generic barriers on all queues if no events were passed to piEnqueueEventsWaitWithBarrier.

@steffenlarsen
[SYCL][L0] Fix piEnqueueEventsWaitWithBarrier for L0
5c23ab5 
When calling piEnqueueEventsWaitWithBarrier while using the level zero
backend, the barrier is only inserted into an available command list.
However, this leaves other command lists unaffected.

This commit fixes this unexpected behavior by making later command-lists
insert a barrier on events from piEnqueueEventsWaitWithBarrier until
they finish.

The resulting event will be either the result of a single barrier, if
events were passed to piEnqueueEventsWaitWithBarrier, or a wait on
generic barriers on all queues if no events were passed to
piEnqueueEventsWaitWithBarrier.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen
Fix formatting
142918f 
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen
Fix depdency refs and simplify solution
bddf1d7 
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen
More comments
829d31e 
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
steffenlarsen added a commit to steffenlarsen/llvm-test-suite that referenced this pull request Jun 27, 2022
@steffenlarsen
[SYCL] Add regression test for barriers with active work
c97445a 
The level zero backend had a bug that caused barriers to not correctly
apply barriers to the entire queue. This is fixed with
intel/llvm#6359 and
SYCL/Regression/barrier_with_work.cpp is added as a regression test for
barriers with active work.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen steffenlarsen mentioned this pull request Jun 27, 2022
Merged
@steffenlarsen steffenlarsen marked this pull request as ready for review June 27, 2022 13:52
@steffenlarsen steffenlarsen requested a review from a team as a code owner June 27, 2022 13:52
@steffenlarsen
Copy link
Contributor Author

/verify with intel/llvm-test-suite#1067

smaslov-intel
smaslov-intel reviewed Jun 28, 2022
View reviewed changes
smaslov-intel
smaslov-intel reviewed Jun 28, 2022
View reviewed changes
smaslov-intel
smaslov-intel reviewed Jun 28, 2022
View reviewed changes
@steffenlarsen
Inline insertBarrier
99fa396 
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen
Add explanation about why we need to force command queues
bfb48c0 
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen
Allow batching
440880e 
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen
Make sure execution also knows batching is allowed
ad0fdee 
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen
Fix formatting
52c0ae4 
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen
Use the right batch argument when getting cmdlist
71971fb 
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@smaslov-intel
Copy link
Contributor

Have you seen the failing testing?

@steffenlarsen
Copy link
Contributor Author

Have you seen the failing testing?

Yes. One was caused by the changes using the wrong argument for batching. The remaining issue I will need to debug further.

smaslov-intel
smaslov-intel reviewed Jul 6, 2022
View reviewed changes
smaslov-intel
smaslov-intel reviewed Jul 6, 2022
View reviewed changes
smaslov-intel
smaslov-intel reviewed Jul 6, 2022
View reviewed changes
smaslov-intel
smaslov-intel reviewed Jul 6, 2022
View reviewed changes
smaslov-intel
smaslov-intel reviewed Jul 6, 2022
View reviewed changes
@steffenlarsen
Address comments, add guard, fix bug
9e9ffff 
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen steffenlarsen requested a review from a team as a code owner July 7, 2022 22:24
@steffenlarsen
Copy link
Contributor Author

/verify with intel/llvm-test-suite#1067

@steffenlarsen
Copy link
Contributor Author

Documentation build failure is a known problem. @v-klochkov - Can you confirm?

@v-klochkov
Copy link
Contributor

Documentation build failure is a known problem. @v-klochkov - Can you confirm?

It is known in our internal workspaces, but hasn't been seen before in intel/llvm workspace. Did we have some pulldown from llvm.org to intel/llvm recently?
If so, I can take care about this here.

@pvchupin
Copy link
Contributor

pvchupin commented Jul 7, 2022 •
edited
Loading

Did we have some pulldown from llvm.org to intel/llvm recently? If so, I can take care about this here.

@v-klochkov, it was merged yesterday: #6358

@v-klochkov
Copy link
Contributor

Did we have some pulldown from llvm.org to intel/llvm recently? If so, I can take care about this here.

@v-klochkov, it was merged yesterday: #6358

Ok, thank you. I uploaded the fix for doc: #6417

@steffenlarsen
Copy link
Contributor Author

/verify with intel/llvm-test-suite#1067

@bader bader merged commit d01371b into intel:sycl Jul 11, 2022
@masterleinad masterleinad mentioned this pull request Sep 29, 2022
Merged
steffenlarsen added a commit to intel/llvm-test-suite that referenced this pull request Oct 17, 2022
@steffenlarsen
[SYCL] Add regression test for barriers with active work (#1067)
f79467e 
The level zero backend had a bug that caused barriers to not correctly
apply barriers to the entire queue. This is fixed with
intel/llvm#6359 and
SYCL/Regression/barrier_with_work.cpp is added as a regression test for
barriers with active work.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
@steffenlarsen
[SYCL] Add regression test for barriers with active work (intel/llvm-…
f475293 
…test-suite#1067)

The level zero backend had a bug that caused barriers to not correctly
apply barriers to the entire queue. This is fixed with
intel#6359 and
SYCL/Regression/barrier_with_work.cpp is added as a regression test for
barriers with active work.

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

@bader bader bader approved these changes

@smaslov-intel smaslov-intel smaslov-intel approved these changes

@againull againull Awaiting requested review from againull

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
@steffenlarsen @smaslov-intel @v-klochkov @pvchupin @bader