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] Test for fix of linked alloca's deps #1470

Merged
merged 16 commits into from
Aug 3, 2020
Merged

[SYCL] Test for fix of linked alloca's deps #1470

merged 16 commits into from
Aug 3, 2020

Conversation

s-kanaev
Copy link
Contributor

@s-kanaev s-kanaev commented Apr 3, 2020 •
edited
Loading

The tests provided here are for this case: https://github.com/intel/llvm/blob/sycl/sycl/source/detail/scheduler/graph_builder.cpp#L611.

The fix was introduced with #1471.

Use case fixed by the fix. Imagine us having two device queues (Q1 and Q2) and a single buffer B. We initialize the buffer with host accessor. Also lets have two kernels: kernel K1 executed at Q1; and kernel K2 which executes at Q2. K1 writes to B, K2 reads from B. After submitting K1 to Q1 there will also be an AllocaCommand A1 which allocates buffer on device. K1 depends on A1 via memory object B. After submitting K2 to Q2 there will be another AllocaCommand A2 (for queue Q2 and its device). K2 will depend on A2. A2, however, should depend on both A1 and K1. A2->K1 dependency eliminates data race.

@s-kanaev s-kanaev requested a review from bader as a code owner April 3, 2020 13:19
romanovvlad
romanovvlad requested changes Apr 3, 2020
View reviewed changes
Copy link
Contributor

@romanovvlad romanovvlad left a comment

Choose a reason for hiding this comment

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

Please, add a test.

@s-kanaev
Copy link
Contributor Author

s-kanaev commented Apr 8, 2020

Please, add a test.

Done

@s-kanaev s-kanaev requested a review from romanovvlad April 8, 2020 14:09
Sergey Kanaev added 2 commits April 9, 2020 09:47
[SYCL] Fix test
49a8909 
Signed-off-by: Sergey Kanaev <[email protected]>
[SYCL] Fix test
462ee07 
Signed-off-by: Sergey Kanaev <[email protected]>
romanovvlad
romanovvlad requested changes Apr 9, 2020
View reviewed changes
sycl/test/scheduler/LinkedAlloca.cpp Outdated Show resolved Hide resolved
@s-kanaev s-kanaev marked this pull request as draft June 15, 2020 13:49
Sergey Kanaev added 3 commits June 17, 2020 12:30
[SYCL] Add unit-test
704fdb0 
Signed-off-by: Sergey Kanaev <[email protected]>
[SYCL] Fix lit test
0abc928 
Signed-off-by: Sergey Kanaev <[email protected]>
[SYCL] Fix style issue
e84cad7 
Signed-off-by: Sergey Kanaev <[email protected]>
@s-kanaev s-kanaev changed the title [SYCL] Fix linked AllocaCmd dependencies [SYCL] Test for fix of linked alloca's deps Jun 17, 2020
[SYCL] Fix style issue
fa0dda4 
Signed-off-by: Sergey Kanaev <[email protected]>
@s-kanaev s-kanaev marked this pull request as ready for review June 17, 2020 10:55
@s-kanaev s-kanaev requested a review from a team as a code owner June 17, 2020 10:55
@s-kanaev s-kanaev requested a review from romanovvlad June 17, 2020 10:55
alexbatashev
alexbatashev reviewed Jun 17, 2020
View reviewed changes
Copy link
Contributor

@alexbatashev alexbatashev left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd like @romanovvlad to take a look

sycl/unittests/scheduler/LinkedAllocaDependencies.cpp Outdated Show resolved Hide resolved
Sergey Kanaev added 2 commits June 17, 2020 15:59
[SYCL] Fix lit test
658b057 
Signed-off-by: Sergey Kanaev <[email protected]>
[SYCL] Apply suggestion
107fe7f 
Signed-off-by: Sergey Kanaev <[email protected]>
@keryell keryell mentioned this pull request Jun 19, 2020
Open
@bader bader requested a review from romanovvlad June 25, 2020 15:27
@bader bader removed their request for review July 9, 2020 18:55
@s-kanaev
Copy link
Contributor Author

/summary:run

Sergey Kanaev added 2 commits July 30, 2020 15:57
@s-kanaev s-kanaev requested a review from alexbatashev July 31, 2020 11:57
@s-kanaev
Copy link
Contributor Author

/summary:run

@s-kanaev
Copy link
Contributor Author

@romanovvlad, please, review

@bader bader merged commit 91a9796 into intel:sycl Aug 3, 2020
@s-kanaev s-kanaev deleted the private/s-kanaev/fix-linked-allocacmd-deps branch September 2, 2020 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romanovvlad romanovvlad romanovvlad approved these changes

@alexbatashev alexbatashev Awaiting requested review from alexbatashev

Assignees

@romanovvlad romanovvlad

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@s-kanaev @alexbatashev @romanovvlad @bader