Skip to content

Commit 00fa367

Browse files
[SYCL][Graph] Bug fixes for hanging tests and urCommandBufferAppendKernelLaunchExp (#11232)
This patch targets to address two issues found in the testing: - Test hangs: This patch serializes the graph submissions in the tests to prevent hangs when submitting multiple graphs. - Issue adding a kernel to the command buffer in `urCommandBufferAppendKernelLaunchExp`. --------- Co-authored-by: Maxime France-Pillois <[email protected]>
1 parent 8c4cad4 commit 00fa367

20 files changed

+173
-120
lines changed

sycl/plugins/unified_runtime/ur/adapters/level_zero/command_buffer.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -474,8 +474,8 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
474474
// the kernel argument declared as a pointer to global or constant memory.
475475
char **ZeHandlePtr = nullptr;
476476
if (Arg.Value) {
477-
// TODO: Not sure of the implication of not passing a device pointer here
478-
UR_CALL(Arg.Value->getZeHandlePtr(ZeHandlePtr, Arg.AccessMode));
477+
UR_CALL(Arg.Value->getZeHandlePtr(ZeHandlePtr, Arg.AccessMode,
478+
CommandBuffer->Device));
479479
}
480480
ZE2UR_CALL(zeKernelSetArgumentValue,
481481
(Kernel->ZeKernel, Arg.Index, Arg.Size, ZeHandlePtr));

sycl/source/detail/graph_impl.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,11 @@ exec_graph_impl::enqueue(const std::shared_ptr<sycl::detail::queue_impl> &Queue,
583583
sycl::detail::EventImplPtr NewEvent;
584584

585585
if (CommandBuffer) {
586+
if (!previousSubmissionCompleted()) {
587+
throw sycl::exception(make_error_code(errc::invalid),
588+
"This Graph cannot be submitted at the moment "
589+
"because the previous run has not yet completed.");
590+
}
586591
NewEvent = CreateNewEvent();
587592
sycl::detail::pi::PiEvent *OutEvent = &NewEvent->getHandleRef();
588593
// Merge requirements from the nodes into requirements (if any) from the

sycl/source/detail/graph_impl.hpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <sycl/handler.hpp>
1515

1616
#include <detail/accessor_impl.hpp>
17+
#include <detail/event_impl.hpp>
1718
#include <detail/kernel_impl.hpp>
1819

1920
#include <cstring>
@@ -670,6 +671,20 @@ class exec_graph_impl {
670671
/// @return pointer to the graph_impl MGraphImpl
671672
const std::shared_ptr<graph_impl> &getGraphImpl() const { return MGraphImpl; }
672673

674+
/// Checks if the previous submissions of this graph have been completed
675+
/// This function checks the status of events associated to the previous graph
676+
/// submissions.
677+
/// @return true if all previous submissions have been completed, false
678+
/// otherwise.
679+
bool previousSubmissionCompleted() const {
680+
for (auto Event : MExecutionEvents) {
681+
if (!Event->isCompleted()) {
682+
return false;
683+
}
684+
}
685+
return true;
686+
}
687+
673688
private:
674689
/// Create a command-group for the node and add it to command-buffer by going
675690
/// through the scheduler.

sycl/test-e2e/Graph/Explicit/enqueue_ordering.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ int main() {
7070
});
7171
});
7272

73+
E4.wait();
74+
7375
// Buffer elements set to 22
7476
Queue.submit([&](handler &CGH) {
7577
CGH.depends_on(E5);

sycl/test-e2e/Graph/Inputs/add_nodes_after_finalize.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,15 @@ int main() {
6363
CGH.depends_on(Event);
6464
CGH.ext_oneapi_graph(GraphExec);
6565
});
66+
Event.wait();
6667
}
6768

6869
for (unsigned n = 0; n < Iterations; n++) {
6970
Event = Queue.submit([&](handler &CGH) {
7071
CGH.depends_on(Event);
7172
CGH.ext_oneapi_graph(GraphExecAdditional);
7273
});
74+
Event.wait();
7375
}
7476

7577
Queue.wait_and_throw();

sycl/test-e2e/Graph/Inputs/basic_buffer.cpp

Lines changed: 36 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -18,42 +18,43 @@ int main() {
1818
calculate_reference_data(Iterations, Size, ReferenceA, ReferenceB,
1919
ReferenceC);
2020

21-
buffer<T> BufferA{DataA.data(), range<1>{DataA.size()}};
22-
BufferA.set_write_back(false);
23-
buffer<T> BufferB{DataB.data(), range<1>{DataB.size()}};
24-
BufferB.set_write_back(false);
25-
buffer<T> BufferC{DataC.data(), range<1>{DataC.size()}};
26-
BufferC.set_write_back(false);
27-
{
28-
exp_ext::command_graph Graph{
29-
Queue.get_context(),
30-
Queue.get_device(),
31-
{exp_ext::property::graph::assume_buffer_outlives_graph{}}};
32-
33-
// Add commands to graph
34-
add_nodes(Graph, Queue, Size, BufferA, BufferB, BufferC);
35-
36-
auto GraphExec = Graph.finalize();
37-
38-
event Event;
39-
for (unsigned n = 0; n < Iterations; n++) {
40-
Event = Queue.submit([&](handler &CGH) {
41-
CGH.depends_on(Event);
42-
CGH.ext_oneapi_graph(GraphExec);
43-
});
44-
}
45-
Queue.wait_and_throw();
46-
}
47-
48-
host_accessor HostAccA(BufferA);
49-
host_accessor HostAccB(BufferB);
50-
host_accessor HostAccC(BufferC);
51-
52-
for (size_t i = 0; i < Size; i++) {
53-
assert(ReferenceA[i] == HostAccA[i]);
54-
assert(ReferenceB[i] == HostAccB[i]);
55-
assert(ReferenceC[i] == HostAccC[i]);
21+
buffer<T> BufferA{DataA.data(), range<1>{DataA.size()}};
22+
BufferA.set_write_back(false);
23+
buffer<T> BufferB{DataB.data(), range<1>{DataB.size()}};
24+
BufferB.set_write_back(false);
25+
buffer<T> BufferC{DataC.data(), range<1>{DataC.size()}};
26+
BufferC.set_write_back(false);
27+
{
28+
exp_ext::command_graph Graph{
29+
Queue.get_context(),
30+
Queue.get_device(),
31+
{exp_ext::property::graph::assume_buffer_outlives_graph{}}};
32+
33+
// Add commands to graph
34+
add_nodes(Graph, Queue, Size, BufferA, BufferB, BufferC);
35+
36+
auto GraphExec = Graph.finalize();
37+
38+
event Event;
39+
for (unsigned n = 0; n < Iterations; n++) {
40+
Event = Queue.submit([&](handler &CGH) {
41+
CGH.depends_on(Event);
42+
CGH.ext_oneapi_graph(GraphExec);
43+
});
44+
Event.wait();
5645
}
46+
Queue.wait_and_throw();
47+
}
48+
49+
host_accessor HostAccA(BufferA);
50+
host_accessor HostAccB(BufferB);
51+
host_accessor HostAccC(BufferC);
52+
53+
for (size_t i = 0; i < Size; i++) {
54+
assert(ReferenceA[i] == HostAccA[i]);
55+
assert(ReferenceB[i] == HostAccB[i]);
56+
assert(ReferenceC[i] == HostAccC[i]);
57+
}
5758

5859
return 0;
5960
}

sycl/test-e2e/Graph/Inputs/basic_usm.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ int main() {
4949
CGH.depends_on(Event);
5050
CGH.ext_oneapi_graph(GraphExec);
5151
});
52+
Event.wait();
5253
}
5354

5455
Queue.wait_and_throw();

sycl/test-e2e/Graph/Inputs/basic_usm_host.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ int main() {
4444
CGH.depends_on(Event);
4545
CGH.ext_oneapi_graph(GraphExec);
4646
});
47+
Event.wait();
4748
}
4849

4950
Queue.wait_and_throw();

sycl/test-e2e/Graph/Inputs/basic_usm_mixed.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ int main() {
4747
CGH.depends_on(Event);
4848
CGH.ext_oneapi_graph(GraphExec);
4949
});
50+
Event.wait();
5051
}
5152

5253
Queue.wait_and_throw();

sycl/test-e2e/Graph/Inputs/basic_usm_shared.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ int main() {
4444
CGH.depends_on(Event);
4545
CGH.ext_oneapi_graph(GraphExec);
4646
});
47+
Event.wait();
4748
}
4849

4950
Queue.wait_and_throw();

0 commit comments

Comments
 (0)