Skip to content

[SYCL] Avoid datarace when waiting on the same event in multiple threads #1803

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

Merged
merged 1 commit into from
Jun 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions sycl/source/detail/event_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,15 @@ class event_impl {

/// Returns command that is associated with the event.
///
/// Scheduler mutex must be locked in read mode when this is called.
///
/// @return a generic pointer to Command object instance.
void *getCommand() { return MCommand; }

/// Associates this event with the command.
///
/// Scheduler mutex must be locked in write mode when this is called.
///
/// @param Command is a generic pointer to Command object instance.
void setCommand(void *Command) { MCommand = Command; }

Expand Down
6 changes: 5 additions & 1 deletion sycl/source/detail/scheduler/graph_processor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ Scheduler::GraphProcessor::getWaitList(EventImplPtr Event) {

void Scheduler::GraphProcessor::waitForEvent(EventImplPtr Event) {
Command *Cmd = getCommand(Event);
assert(Cmd && "Event has no associated command?");
// Command can be nullptr if user creates cl::sycl::event explicitly or the
// event has been waited on by another thread
if (!Cmd)
return;

EnqueueResultT Res;
bool Enqueued = enqueueCommand(Cmd, Res, BLOCKING);
if (!Enqueued && EnqueueResultT::SyclEnqueueFailed == Res.MResult)
Expand Down
1 change: 1 addition & 0 deletions sycl/unittests/scheduler/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ add_sycl_unittest(SchedulerTests OBJECT
LeafLimit.cpp
MemObjCommandCleanup.cpp
CommandsWaitForEvents.cpp
WaitAfterCleanup.cpp
utils.cpp
)
28 changes: 28 additions & 0 deletions sycl/unittests/scheduler/WaitAfterCleanup.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//==------------ WaitAfterCleanup.cpp ---- Scheduler unit tests ------------==//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include "SchedulerTest.hpp"
#include "SchedulerTestUtils.hpp"

using namespace cl::sycl;

TEST_F(SchedulerTest, WaitAfterCleanup) {
auto Cmd = new MockCommand(detail::getSyclObjImpl(MQueue));
auto Event = Cmd->getEvent();
ASSERT_NE(Event, nullptr) << "Command must have an event\n";

detail::Scheduler::getInstance().waitForEvent(Event);
ASSERT_EQ(Event->getCommand(), Cmd)
<< "Command should not have been cleaned up yet\n";

detail::Scheduler::getInstance().cleanupFinishedCommands(Event);
ASSERT_EQ(Event->getCommand(), nullptr)
<< "Command should have been cleaned up\n";

detail::Scheduler::getInstance().waitForEvent(Event);
}