Skip to content

Cperkins buffer fail fail invest #65

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

Draft
wants to merge 3 commits into
base: sycl
Choose a base branch
from
Draft

Cperkins buffer fail fail invest #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5d0388b
interim
cperkinsintel Mar 31, 2025
7090dac
seems to fix, but needs to be checked
cperkinsintel Apr 1, 2025
96e423b
fix?
cperkinsintel Apr 3, 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
4 changes: 4 additions & 0 deletions sycl/source/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
#2. Use AddLLVM to modify the build and access config options
#cmake_policy(SET CMP0057 NEW)
#include(AddLLVM)

set(CMAKE_BUILD_TYPE Debug)


configure_file(
${CMAKE_CURRENT_SOURCE_DIR}/version.rc.in
${CMAKE_CURRENT_BINARY_DIR}/version.rc
Expand Down
4 changes: 3 additions & 1 deletion sycl/source/detail/scheduler/graph_builder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,8 +486,10 @@ Scheduler::GraphBuilder::addCopyBack(Requirement *Req,

std::vector<Command *> ToCleanUp;
for (Command *Dep : Deps) {
if (Dep->MEnqueueStatus == EnqueueResultT::SyclEnqueueFailed)
if (Dep->MEnqueueStatus == EnqueueResultT::SyclEnqueueFailed) {
std::cout << "graph_builder:490 says HELLO!!" << std::endl;
continue;
}

Command *ConnCmd = MemCpyCmd->addDep(
DepDesc{Dep, MemCpyCmd->getRequirement(), SrcAllocaCmd}, ToCleanUp);
Expand Down
7 changes: 7 additions & 0 deletions sycl/source/detail/scheduler/graph_processor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,13 @@ bool Scheduler::GraphProcessor::enqueueCommand(
return false;
}

// CP -- FAIL TWO
// Reset enqueue status if reattempting
if(Cmd->MEnqueueStatus == EnqueueResultT::SyclEnqueueFailed){
Cmd->MEnqueueStatus = EnqueueResultT::SyclEnqueueReady;
//std::cout << "CP FAIL TWO. EnqueueResult/Cmd/Err: " << EnqueueResult.MResult << "/" << (long)EnqueueResult.MCmd << "/" << EnqueueResult.MErrCode << std::endl;
}

// Recursively enqueue all the implicit + explicit backend level dependencies
// first and exit immediately if any of the commands cannot be enqueued.
for (const EventImplPtr &Event : Cmd->getPreparedDepsEvents()) {
Expand Down
42 changes: 28 additions & 14 deletions sycl/source/detail/scheduler/scheduler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,31 +52,35 @@ void Scheduler::waitForRecordToFinish(MemObjRecord *Record,
#endif
std::vector<Command *> ToCleanUp;
for (Command *Cmd : Record->MReadLeaves) {
if (Cmd->MEnqueueStatus == EnqueueResultT::SyclEnqueueFailed)
if (Cmd->MEnqueueStatus == EnqueueResultT::SyclEnqueueFailed) {
std::cout << "scheduler.cpp:56 says HELLO!" << std::endl;
continue;
}

EnqueueResultT Res;
bool Enqueued =
GraphProcessor::enqueueCommand(Cmd, GraphReadLock, Res, ToCleanUp, Cmd);
if (!Enqueued && EnqueueResultT::SyclEnqueueFailed == Res.MResult)
throw exception(make_error_code(errc::runtime),
"Enqueue process failed.");
"1- Enqueue process failed.");
#ifdef XPTI_ENABLE_INSTRUMENTATION
// Capture the dependencies
DepCommands.insert(Cmd);
#endif
GraphProcessor::waitForEvent(Cmd->getEvent(), GraphReadLock, ToCleanUp);
}
for (Command *Cmd : Record->MWriteLeaves) {
if (Cmd->MEnqueueStatus == EnqueueResultT::SyclEnqueueFailed)
if (Cmd->MEnqueueStatus == EnqueueResultT::SyclEnqueueFailed) {
std::cout << "scheduler.cpp:74 says HELLO!!" << std::endl;
continue;
}

EnqueueResultT Res;
bool Enqueued =
GraphProcessor::enqueueCommand(Cmd, GraphReadLock, Res, ToCleanUp, Cmd);
if (!Enqueued && EnqueueResultT::SyclEnqueueFailed == Res.MResult)
throw exception(make_error_code(errc::runtime),
"Enqueue process failed.");
"2- Enqueue process failed.");
#ifdef XPTI_ENABLE_INSTRUMENTATION
DepCommands.insert(Cmd);
#endif
Expand All @@ -89,7 +93,7 @@ void Scheduler::waitForRecordToFinish(MemObjRecord *Record,
Res, ToCleanUp, ReleaseCmd);
if (!Enqueued && EnqueueResultT::SyclEnqueueFailed == Res.MResult)
throw exception(make_error_code(errc::runtime),
"Enqueue process failed.");
"3- Enqueue process failed.");
#ifdef XPTI_ENABLE_INSTRUMENTATION
// Report these dependencies to the Command so these dependencies can be
// reported as edges
Expand Down Expand Up @@ -156,6 +160,16 @@ void Scheduler::enqueueCommandForCG(EventImplPtr NewEvent,
bool Enqueued;

auto CleanUp = [&]() {

// CP -- FAIL ONE
// restore the enqueue status
// this fixes the bug where the buffer is not re-usable.
// BUT, ironically, it reintroduces the other scheduler failure I fixed,
// where exceptions lead to memory leaks.
// if(NewCmd)
// NewCmd->MEnqueueStatus = EnqueueResultT::SyclEnqueueReady;
//std::cout << "CleanUp Hit! " << NewCmd->MMarkedForCleanup << std::endl;

if (NewCmd && (NewCmd->MDeps.size() == 0 && NewCmd->MUsers.size() == 0)) {
if (NewEvent) {
NewEvent->setCommand(nullptr);
Expand Down Expand Up @@ -189,7 +203,7 @@ void Scheduler::enqueueCommandForCG(EventImplPtr NewEvent,
NewCmd, Lock, Res, ToCleanUp, NewCmd, Blocking);
if (!Enqueued && EnqueueResultT::SyclEnqueueFailed == Res.MResult)
throw exception(make_error_code(errc::runtime),
"Enqueue process failed.");
"4- Enqueue process failed.");
} catch (...) {
// enqueueCommand() func and if statement above may throw an exception,
// so destroy required resources to avoid memory leak
Expand Down Expand Up @@ -230,7 +244,7 @@ EventImplPtr Scheduler::addCopyBack(Requirement *Req) {
if (!Enqueued && EnqueueResultT::SyclEnqueueFailed == Res.MResult) {
CopyBackCmdsFailed |= Res.MCmd == Cmd;
throw exception(make_error_code(errc::runtime),
"Enqueue process failed.");
"5- Enqueue process failed.");
}
}

Expand All @@ -239,7 +253,7 @@ EventImplPtr Scheduler::addCopyBack(Requirement *Req) {
if (!Enqueued && EnqueueResultT::SyclEnqueueFailed == Res.MResult) {
CopyBackCmdsFailed |= Res.MCmd == NewCmd;
throw exception(make_error_code(errc::runtime),
"Enqueue process failed.");
"6- Enqueue process failed.");
}
} catch (...) {
if (CopyBackCmdsFailed) {
Expand Down Expand Up @@ -323,15 +337,15 @@ EventImplPtr Scheduler::addHostAccessor(Requirement *Req) {
Enqueued = GraphProcessor::enqueueCommand(Cmd, Lock, Res, ToCleanUp, Cmd);
if (!Enqueued && EnqueueResultT::SyclEnqueueFailed == Res.MResult)
throw exception(make_error_code(errc::runtime),
"Enqueue process failed.");
"7- Enqueue process failed.");
}

if (Command *NewCmd = static_cast<Command *>(NewCmdEvent->getCommand())) {
Enqueued =
GraphProcessor::enqueueCommand(NewCmd, Lock, Res, ToCleanUp, NewCmd);
if (!Enqueued && EnqueueResultT::SyclEnqueueFailed == Res.MResult)
throw exception(make_error_code(errc::runtime),
"Enqueue process failed.");
"8- Enqueue process failed.");
}
}

Expand Down Expand Up @@ -366,7 +380,7 @@ void Scheduler::enqueueLeavesOfReqUnlocked(const Requirement *const Req,
ToCleanUp, Cmd);
if (!Enqueued && EnqueueResultT::SyclEnqueueFailed == Res.MResult)
throw exception(make_error_code(errc::runtime),
"Enqueue process failed.");
"9- Enqueue process failed.");
}
};

Expand All @@ -386,7 +400,7 @@ void Scheduler::enqueueUnblockedCommands(
GraphProcessor::enqueueCommand(Cmd, GraphReadLock, Res, ToCleanUp, Cmd);
if (!Enqueued && EnqueueResultT::SyclEnqueueFailed == Res.MResult)
throw exception(make_error_code(errc::runtime),
"Enqueue process failed.");
"10- Enqueue process failed.");
}
}

Expand Down Expand Up @@ -632,15 +646,15 @@ EventImplPtr Scheduler::addCommandGraphUpdate(
Enqueued = GraphProcessor::enqueueCommand(Cmd, Lock, Res, ToCleanUp, Cmd);
if (!Enqueued && EnqueueResultT::SyclEnqueueFailed == Res.MResult)
throw exception(make_error_code(errc::runtime),
"Enqueue process failed.");
"11- Enqueue process failed.");
}

if (Command *NewCmd = static_cast<Command *>(NewCmdEvent->getCommand())) {
Enqueued =
GraphProcessor::enqueueCommand(NewCmd, Lock, Res, ToCleanUp, NewCmd);
if (!Enqueued && EnqueueResultT::SyclEnqueueFailed == Res.MResult)
throw exception(make_error_code(errc::runtime),
"Enqueue process failed.");
"12- Enqueue process failed.");
}
}

Expand Down
16 changes: 8 additions & 8 deletions sycl/unittests/scheduler/FailedCommands.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ TEST_F(SchedulerTest, FailedDependency) {
queue Queue(context(Plt), default_selector_v);

detail::Requirement MockReq = getMockRequirement();
MockCommand MDep(detail::getSyclObjImpl(Queue));
MockCommand MUser(detail::getSyclObjImpl(Queue));
MDep.addUser(&MUser);
MockCommand MDepFail(false, detail::getSyclObjImpl(Queue)); // <-- will fail to enqueue
MockCommand MUser(detail::getSyclObjImpl(Queue));
MDepFail.addUser(&MUser);
std::vector<detail::Command *> ToCleanUp;
(void)MUser.addDep(detail::DepDesc{&MDep, &MockReq, nullptr}, ToCleanUp);
(void)MUser.addDep(detail::DepDesc{&MDepFail, &MockReq, nullptr}, ToCleanUp);
MUser.MEnqueueStatus = detail::EnqueueResultT::SyclEnqueueReady;
MDep.MEnqueueStatus = detail::EnqueueResultT::SyclEnqueueFailed;
MDepFail.MEnqueueStatus = detail::EnqueueResultT::SyclEnqueueReady;

MockScheduler MS;
auto Lock = MS.acquireGraphReadLock();
Expand All @@ -35,13 +35,13 @@ TEST_F(SchedulerTest, FailedDependency) {
MockScheduler::enqueueCommand(&MUser, Res, detail::NON_BLOCKING);

ASSERT_FALSE(Enqueued) << "Enqueue process must fail\n";
ASSERT_EQ(Res.MCmd, &MDep) << "Wrong failed command\n";
ASSERT_EQ(Res.MCmd, &MDepFail) << "Wrong failed command\n";
ASSERT_EQ(Res.MResult, detail::EnqueueResultT::SyclEnqueueFailed)
<< "Enqueue process must fail\n";
ASSERT_EQ(MUser.MEnqueueStatus, detail::EnqueueResultT::SyclEnqueueReady)
<< "MUser shouldn't be marked as failed\n";
ASSERT_EQ(MDep.MEnqueueStatus, detail::EnqueueResultT::SyclEnqueueFailed)
<< "MDep should be marked as failed\n";
ASSERT_EQ(MDepFail.MEnqueueStatus, detail::EnqueueResultT::SyclEnqueueFailed)
<< "MDepFail should be marked as failed\n";
}

void RunWithFailedCommandsAndCheck(bool SyncExceptionExpected,
Expand Down
17 changes: 17 additions & 0 deletions sycl/unittests/scheduler/SchedulerTestUtils.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ class MockCommand : public sycl::detail::Command {
EXPECT_CALL(*this, enqueue).Times(AnyNumber());
}

MockCommand(bool, sycl::detail::QueueImplPtr Queue,
sycl::detail::Command::CommandType Type = sycl::detail::Command::RUN_CG)
: Command{Type, Queue}, MRequirement{std::move(getMockRequirement())} {
using namespace testing;
ON_CALL(*this, enqueue)
.WillByDefault(Invoke(this, &MockCommand::enqueueFail));
EXPECT_CALL(*this, enqueue).Times(AnyNumber());
}

void printDot(std::ostream &) const override {}
void emitInstrumentationData() override {}

Expand All @@ -70,6 +79,14 @@ class MockCommand : public sycl::detail::Command {
std::vector<sycl::detail::Command *> &ToCleanUp) {
return sycl::detail::Command::enqueue(EnqueueResult, Blocking, ToCleanUp);
}
bool enqueueFail(sycl::detail::EnqueueResultT &EnqueueResult,
sycl::detail::BlockingT Blocking,
std::vector<sycl::detail::Command *> &ToCleanUp) {
this->MEnqueueStatus = sycl::detail::EnqueueResultT::SyclEnqueueFailed;
EnqueueResult = {sycl::detail::EnqueueResultT::SyclEnqueueFailed, this};
ToCleanUp.push_back(this);
return false;
}

ur_result_t MRetVal = UR_RESULT_SUCCESS;

Expand Down
Loading