[SYCL] Build proper barrier deps if host task is involved in pipeline #13094

KseniyaTikhomirova · 2024-03-21T17:21:39Z

No description provided.

This reverts commit 2f56926.

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

…iEvent Signed-off-by: Tikhomirova, Kseniya <[email protected]>

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova · 2024-03-21T17:22:04Z

to do:

update windows symbols -Done
implement UT - Done

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

sergey-semenov

LGTM overall, just some minor comments.

sycl/include/sycl/handler.hpp

sycl/source/detail/queue_impl.hpp

sycl/source/detail/queue_impl.cpp

sycl/source/detail/queue_impl.hpp

sycl/source/detail/event_impl.hpp

sycl/source/handler.cpp

sycl/unittests/scheduler/HostTaskAndBarrier.cpp

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

bader · 2024-09-09T22:20:36Z

sycl/source/detail/queue_impl.hpp

+  // Helps to manage host tasks presence in scenario with barrier usage.
+  // Approach that tracks almost all tasks to provide barrier sync for both pi
+  // tasks and host tasks is applicable for out of order queues only. No-op
+  // for in order ones.
+  void tryToResetEnqueuedBarrierDep(const EventImplPtr &EnqueuedBarrierEvent);


This seems to be a dead code.

bader · 2024-09-09T22:22:00Z

sycl/source/detail/queue_impl.hpp

+
+  // Called on host task completion that could block some kernels from enqueue.
+  // Approach that tracks almost all tasks to provide barrier sync for both pi
+  // tasks and host tasks is applicable for out of order queues only. Not neede


neede -> needed

bader · 2024-09-09T22:27:09Z

sycl/source/detail/queue_impl.hpp

+  // Called on host task completion that could block some kernels from enqueue.
+  // Approach that tracks almost all tasks to provide barrier sync for both pi
+  // tasks and host tasks is applicable for out of order queues only. Not neede
+  // for in order ones.


Considering that the backend is not aware of host tasks, how in-order property is provided for host tasks?

bader · 2024-09-09T22:31:23Z

sycl/unittests/scheduler/HostTaskAndBarrier.cpp

+  sycl::event HTEvent = AddTask(TestCGType::HOST_TASK);
+  EventImplPtr HostTaskEventImpl = sycl::detail::getSyclObjImpl(HTEvent);
+  auto HostTaskWaitList = HostTaskEventImpl->getWaitList();
+  ASSERT_EQ(HostTaskWaitList.size(), 0u);


Shouldn't host task to wait for barrier to complete?

bader · 2024-09-09T22:34:34Z

sycl/unittests/scheduler/HostTaskAndBarrier.cpp

+  sycl::event KernelEvent = AddTask(TestCGType::KERNEL_TASK);
+  EventImplPtr KernelEventImpl = sycl::detail::getSyclObjImpl(KernelEvent);
+  auto KernelWaitList = KernelEventImpl->getWaitList();
+  ASSERT_EQ(KernelWaitList.size(), 0u);


Do I understand correctly that the barrier semantics is guaranteed by the backend for the kernel task, so there is not dependency on barrier?

I suggest adding in-order queue test case with code comments describing code logic.

bader · 2024-09-09T22:36:36Z

sycl/unittests/scheduler/HostTaskAndBarrier.cpp

+  sycl::event HTEvent = AddTask(TestCGType::HOST_TASK);
+  EventImplPtr HostTaskEventImpl = sycl::detail::getSyclObjImpl(HTEvent);
+  auto HostTaskWaitList = HostTaskEventImpl->getWaitList();
+  ASSERT_EQ(HostTaskWaitList.size(), 0u);


I would expect to see a dependency on barrier event here (similar to BarrierHandlingWithHostTask test case).

bader · 2024-09-09T23:00:19Z

sycl/unittests/scheduler/InOrderQueueSyncCheck.cpp

+    EXPECT_CALL(MockCGH, depends_on(An<const sycl::detail::EventImplPtr &>()))
+        .Times(1);
+    Queue->finalizeHandler<LimitedHandlerSimulation>(
+        MockCGH, detail::CG::CGTYPE::CodeplayHostTask, Event);


Do we test scenarios with multiple in-order queues, where host task enqueued in one queue and event was passed as an argument to a barrier in another queue?

KseniyaTikhomirova added 8 commits March 15, 2024 06:25

Revert "[SYCL] Fix unused variable warnings (intel#12523)"

9f15ae2

This reverts commit 2f56926.

Extend deps structure

706fc20

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

Adapt changes to current code base. Buils is failing due to producesP…

3ab87d6

…iEvent Signed-off-by: Tikhomirova, Kseniya <[email protected]>

Fix build

ce69929

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

fix braces

280ccb4

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

Fix recursive mutex lock

e6fd2ad

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

fix clang-format

c79e215

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

update linux symbols

5625070

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova requested a review from sergey-semenov March 21, 2024 17:22

Merge branch 'sycl' into barrier_rebase

4d6fb63

KseniyaTikhomirova had a problem deploying to WindowsCILock March 21, 2024 17:53 — with GitHub Actions Failure

KseniyaTikhomirova had a problem deploying to WindowsCILock March 21, 2024 18:29 — with GitHub Actions Failure

KseniyaTikhomirova added 7 commits March 28, 2024 10:23

Merge branch 'sycl' into barrier_rebase

9080bcc

Draft

69c53e9

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

fix idea

2640855

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

Fix logic

6454bc4

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

fix clang format

81e295a

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

Merge branch 'sycl' into barrier_rebase

8c5dd95

Cleanup

45914f9

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova had a problem deploying to WindowsCILock April 12, 2024 14:27 — with GitHub Actions Failure

KseniyaTikhomirova had a problem deploying to WindowsCILock April 12, 2024 14:57 — with GitHub Actions Failure

KseniyaTikhomirova added 2 commits April 15, 2024 09:03

Add unittests

2b47556

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

add test for barrier with wait list

1646766

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova had a problem deploying to WindowsCILock April 15, 2024 17:23 — with GitHub Actions Failure

KseniyaTikhomirova had a problem deploying to WindowsCILock April 15, 2024 17:51 — with GitHub Actions Error

fix issue

7590a5d

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova had a problem deploying to WindowsCILock April 15, 2024 18:39 — with GitHub Actions Failure

KseniyaTikhomirova had a problem deploying to WindowsCILock April 15, 2024 19:29 — with GitHub Actions Failure

FIx clang-format

2bee189

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

Merge branch 'sycl' into barrier_rebase

d249da7

KseniyaTikhomirova had a problem deploying to WindowsCILock April 16, 2024 11:45 — with GitHub Actions Failure

KseniyaTikhomirova had a problem deploying to WindowsCILock April 16, 2024 12:17 — with GitHub Actions Failure

Update Win symbols

df17b54

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova temporarily deployed to WindowsCILock April 18, 2024 16:34 — with GitHub Actions Inactive

KseniyaTikhomirova had a problem deploying to WindowsCILock April 18, 2024 17:02 — with GitHub Actions Failure

Fix seg fault

aed5f81

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova marked this pull request as ready for review April 19, 2024 13:09

KseniyaTikhomirova requested a review from a team as a code owner April 19, 2024 13:09

KseniyaTikhomirova temporarily deployed to WindowsCILock April 19, 2024 13:22 — with GitHub Actions Inactive

KseniyaTikhomirova temporarily deployed to WindowsCILock April 19, 2024 13:51 — with GitHub Actions Inactive

sergey-semenov reviewed Apr 25, 2024

View reviewed changes

KseniyaTikhomirova added 2 commits May 6, 2024 05:29

fix comments, part1

a9eb8a3

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

Merge branch 'sycl' into barrier_rebase

166ffd4

KseniyaTikhomirova had a problem deploying to WindowsCILock May 6, 2024 12:29 — with GitHub Actions Error

fix comments, part 2

cfb2ef0

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova temporarily deployed to WindowsCILock May 6, 2024 12:40 — with GitHub Actions Inactive

KseniyaTikhomirova requested a review from sergey-semenov May 6, 2024 12:40

KseniyaTikhomirova temporarily deployed to WindowsCILock May 6, 2024 13:10 — with GitHub Actions Inactive

sergey-semenov approved these changes May 7, 2024

View reviewed changes

sergey-semenov merged commit bcf5385 into intel:sycl May 8, 2024

bader reviewed Sep 9, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SYCL] Build proper barrier deps if host task is involved in pipeline #13094

[SYCL] Build proper barrier deps if host task is involved in pipeline #13094

Uh oh!

KseniyaTikhomirova commented Mar 21, 2024

Uh oh!

KseniyaTikhomirova commented Mar 21, 2024 •

edited

Loading

Uh oh!

sergey-semenov left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bader Sep 9, 2024

Uh oh!

bader Sep 9, 2024

Uh oh!

bader Sep 9, 2024

Uh oh!

bader Sep 9, 2024

Uh oh!

bader Sep 9, 2024

Uh oh!

bader Sep 9, 2024

Uh oh!

bader Sep 9, 2024

Uh oh!

Uh oh!

[SYCL] Build proper barrier deps if host task is involved in pipeline #13094

[SYCL] Build proper barrier deps if host task is involved in pipeline #13094

Uh oh!

Conversation

KseniyaTikhomirova commented Mar 21, 2024

Uh oh!

KseniyaTikhomirova commented Mar 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sergey-semenov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bader Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

bader Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

bader Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

bader Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

bader Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

bader Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

bader Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

KseniyaTikhomirova commented Mar 21, 2024 •

edited

Loading