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

[SYCL] Extend scope of scheduler bypass to safe to bypass events #16735

Open
wants to merge 7 commits into
base: sycl
Choose a base branch
from
Open

[SYCL] Extend scope of scheduler bypass to safe to bypass events #16735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3af411a
[SYCL] Extend scope of scheduler bypass to safe to bypass events
Alexandr-Konovalov Jan 22, 2025
7991a06
Fix code formatting.
Alexandr-Konovalov Jan 22, 2025
faf3cb3
Update sycl/source/detail/queue_impl.cpp
Alexandr-Konovalov Jan 22, 2025
4f5b484
Fix test-e2e.
Alexandr-Konovalov Jan 29, 2025
787192b
Fix code formatting.
Alexandr-Konovalov Jan 29, 2025
d90b475
Small fixes.
Alexandr-Konovalov Jan 29, 2025
28b0a14
Eliminate excessive copying.
Alexandr-Konovalov Feb 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion sycl/source/detail/queue_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,14 +481,25 @@ event queue_impl::submitMemOpHelper(const std::shared_ptr<queue_impl> &Self,
}

event ResEvent = prepareSYCLEventAssociatedWithQueue(Self);
auto EventImpl = detail::getSyclObjImpl(ResEvent);
const auto &EventImpl = detail::getSyclObjImpl(ResEvent);
{
NestedCallsTracker tracker;
ur_event_handle_t UREvent = nullptr;
MemOpFunc(MemOpArgs..., getUrEvents(ExpandedDepEvents), &UREvent,
EventImpl);
EventImpl->setHandle(UREvent);
EventImpl->setEnqueued();
// connect returned event with dependent events
if (!isInOrder()) {
std::vector<EventImplPtr> &ExpandedDepEventImplPtrs =
EventImpl->getPreparedDepsEvents();
ExpandedDepEventImplPtrs.reserve(ExpandedDepEvents.size());
for (const event &DepEvent : ExpandedDepEvents)
ExpandedDepEventImplPtrs.push_back(
detail::getSyclObjImpl(DepEvent));

EventImpl->cleanDepEventsThroughOneLevel();
}
}

if (isInOrder()) {
Expand Down
4 changes: 2 additions & 2 deletions sycl/source/detail/scheduler/scheduler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ bool CheckEventReadiness(const ContextImplPtr &Context,
}

bool Scheduler::areEventsSafeForSchedulerBypass(
const std::vector<sycl::event> &DepEvents, ContextImplPtr Context) {
const std::vector<sycl::event> &DepEvents, const ContextImplPtr &Context) {

return std::all_of(
DepEvents.begin(), DepEvents.end(), [&Context](const sycl::event &Event) {
Expand All @@ -672,7 +672,7 @@ bool Scheduler::areEventsSafeForSchedulerBypass(
}

bool Scheduler::areEventsSafeForSchedulerBypass(
const std::vector<EventImplPtr> &DepEvents, ContextImplPtr Context) {
const std::vector<EventImplPtr> &DepEvents, const ContextImplPtr &Context) {

return std::all_of(DepEvents.begin(), DepEvents.end(),
[&Context](const EventImplPtr &SyclEventImplPtr) {
Expand Down
4 changes: 2 additions & 2 deletions sycl/source/detail/scheduler/scheduler.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,10 +479,10 @@ class Scheduler {

static bool
areEventsSafeForSchedulerBypass(const std::vector<sycl::event> &DepEvents,
ContextImplPtr Context);
const ContextImplPtr &Context);
static bool
areEventsSafeForSchedulerBypass(const std::vector<EventImplPtr> &DepEvents,
ContextImplPtr Context);
const ContextImplPtr &Context);

protected:
using RWLockT = std::shared_timed_mutex;
Expand Down
11 changes: 7 additions & 4 deletions sycl/source/handler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,10 +468,8 @@ event handler::finalize() {
if (MQueue && !impl->MGraph && !impl->MSubgraphNode &&
!MQueue->getCommandGraph() && !impl->CGData.MRequirements.size() &&
!MStreamStorage.size() &&
(!impl->CGData.MEvents.size() ||
(MQueue->isInOrder() &&
detail::Scheduler::areEventsSafeForSchedulerBypass(
impl->CGData.MEvents, MQueue->getContextImplPtr())))) {
detail::Scheduler::areEventsSafeForSchedulerBypass(
impl->CGData.MEvents, MQueue->getContextImplPtr())) {
steffenlarsen marked this conversation as resolved.
Show resolved Hide resolved
// if user does not add a new dependency to the dependency graph, i.e.
// the graph is not changed, then this faster path is used to submit
// kernel bypassing scheduler and avoiding CommandGroup, Command objects
Expand Down Expand Up @@ -546,6 +544,11 @@ event handler::finalize() {
if (NewEvent->isHost() || NewEvent->getHandle() == nullptr)
NewEvent->setComplete();
NewEvent->setEnqueued();
// connect returned event with dependent events
if (!MQueue->isInOrder()) {
NewEvent->getPreparedDepsEvents() = impl->CGData.MEvents;
NewEvent->cleanDepEventsThroughOneLevel();
}

MLastEvent = detail::createSyclObjFromImpl<event>(NewEvent);
}
Expand Down
6 changes: 0 additions & 6 deletions sycl/test-e2e/XPTI/basic_event_collection_linux.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,6 @@
// CHECK-DAG: from_source : false
// CHECK-DAG: kernel_name : typeinfo name for main::{lambda(sycl::_V1::handler&)#1}::operator()(sycl::_V1::handler&) const::{lambda()#1}
// CHECK-DAG: sycl_device : {{.*}}
// CHECK: Node create
// CHECK-DAG: queue_id : {{.*}}
// CHECK-DAG: kernel_name : virtual_node[{{.*}}]
// CHECK-NEXT: Edge create
// CHECK-DAG: queue_id : {{.*}}
// CHECK-DAG: event : {{.*}}
// CHECK: Task begin
// CHECK-DAG: queue_id : {{.*}}
// CHECK-DAG: sym_line_no : {{.*}}
Expand Down
6 changes: 6 additions & 0 deletions sycl/unittests/scheduler/EnqueueWithDependsOnDeps.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ TEST_F(DependsOnTests, ShortcutFunctionWithWaitList) {
&redefinedextUSMEnqueueMemcpy);
sycl::queue Queue = detail::createSyclObjFromImpl<queue>(QueueDevImpl);

// Mock up an incomplete host task
auto HostTaskEvent =
Queue.submit([&](sycl::handler &cgh) { cgh.host_task([=]() {}); });
std::shared_ptr<detail::event_impl> HostTaskEventImpl =
Expand All @@ -332,6 +333,7 @@ TEST_F(DependsOnTests, ShortcutFunctionWithWaitList) {
ASSERT_NE(Cmd, nullptr);
Cmd->MIsBlockable = true;
Cmd->MEnqueueStatus = detail::EnqueueResultT::SyclEnqueueBlocked;
HostTaskEventImpl->setStateIncomplete();

auto SingleTaskEvent = Queue.submit([&](sycl::handler &cgh) {
cgh.depends_on(HostTaskEvent);
Expand All @@ -341,6 +343,8 @@ TEST_F(DependsOnTests, ShortcutFunctionWithWaitList) {
detail::getSyclObjImpl(SingleTaskEvent);
EXPECT_EQ(SingleTaskEventImpl->getHandle(), nullptr);

// make HostTaskEvent completed, so SingleTaskEvent can be enqueued
HostTaskEventImpl->setComplete();
Cmd->MEnqueueStatus = detail::EnqueueResultT::SyclEnqueueSuccess;
EventsInWaitList.clear();

Expand Down Expand Up @@ -375,6 +379,7 @@ TEST_F(DependsOnTests, BarrierWithWaitList) {
ASSERT_NE(Cmd, nullptr);
Cmd->MIsBlockable = true;
Cmd->MEnqueueStatus = detail::EnqueueResultT::SyclEnqueueBlocked;
HostTaskEventImpl->setStateIncomplete();

auto SingleTaskEvent = Queue.submit([&](sycl::handler &cgh) {
cgh.depends_on(HostTaskEvent);
Expand All @@ -384,6 +389,7 @@ TEST_F(DependsOnTests, BarrierWithWaitList) {
detail::getSyclObjImpl(SingleTaskEvent);
EXPECT_EQ(SingleTaskEventImpl->getHandle(), nullptr);

HostTaskEventImpl->setComplete();
Cmd->MEnqueueStatus = detail::EnqueueResultT::SyclEnqueueSuccess;
EventsInWaitList.clear();

Expand Down
Loading