Skip to content

Commit f2cd2a8

Browse files
[SYCL] Disable in-order queue barrier optimization while profiling (#14123)
Current implementation of profiling info for NOP barriers is inconsistent with other events from the same queue (e.g., if the previous event started after the barrier was submitted). To make them consistent while keeping the optimization, we would need to duplicate the event on our side and make the duplicate check and potentially use profiling info of its previous event. Instead, as the first step, disable the NOP optimization during profiling since profiling is known to incur a performance hit anyway. The proper duplicate event approach can be implemented as a follow up if this causes issues for users. Partially reverts #12949
1 parent e34b7ff commit f2cd2a8

File tree

5 files changed

+50
-79
lines changed

5 files changed

+50
-79
lines changed

sycl/source/detail/event_impl.cpp

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -167,15 +167,11 @@ event_impl::event_impl(sycl::detail::pi::PiEvent Event,
167167
}
168168
}
169169

170-
event_impl::event_impl(const QueueImplPtr &Queue) {
170+
event_impl::event_impl(const QueueImplPtr &Queue)
171+
: MQueue{Queue},
172+
MIsProfilingEnabled{Queue->is_host() || Queue->MIsProfilingEnabled},
173+
MFallbackProfiling{MIsProfilingEnabled && Queue->isProfilingFallback()} {
171174
this->setContextImpl(Queue->getContextImplPtr());
172-
this->associateWithQueue(Queue);
173-
}
174-
175-
void event_impl::associateWithQueue(const QueueImplPtr &Queue) {
176-
MQueue = Queue;
177-
MIsProfilingEnabled = Queue->is_host() || Queue->MIsProfilingEnabled;
178-
MFallbackProfiling = MIsProfilingEnabled && Queue->isProfilingFallback();
179175
if (Queue->is_host()) {
180176
MState.store(HES_NotComplete);
181177
if (Queue->has_property<property::queue::enable_profiling>()) {
@@ -337,11 +333,6 @@ template <>
337333
uint64_t
338334
event_impl::get_profiling_info<info::event_profiling::command_start>() {
339335
checkProfilingPreconditions();
340-
341-
// For nop command start time is equal to submission time.
342-
if (isNOP() && MSubmitTime)
343-
return MSubmitTime;
344-
345336
if (!MHostEvent) {
346337
if (MEvent) {
347338
auto StartTime =
@@ -369,11 +360,6 @@ event_impl::get_profiling_info<info::event_profiling::command_start>() {
369360
template <>
370361
uint64_t event_impl::get_profiling_info<info::event_profiling::command_end>() {
371362
checkProfilingPreconditions();
372-
373-
// For nop command end time is equal to submission time.
374-
if (isNOP() && MSubmitTime)
375-
return MSubmitTime;
376-
377363
if (!MHostEvent) {
378364
if (MEvent) {
379365
auto EndTime =

sycl/source/detail/event_impl.hpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -244,11 +244,6 @@ class event_impl {
244244
MSubmittedQueue = SubmittedQueue;
245245
};
246246

247-
/// Associate event with provided queue.
248-
///
249-
/// @return
250-
void associateWithQueue(const QueueImplPtr &Queue);
251-
252247
/// Indicates if this event is not associated with any command and doesn't
253248
/// have native handle.
254249
///

sycl/source/queue.cpp

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -214,22 +214,7 @@ getBarrierEventForInorderQueueHelper(const detail::QueueImplPtr QueueImpl) {
214214
assert(!QueueImpl->getCommandGraph() &&
215215
"Should not be called in on graph recording.");
216216

217-
auto LastEvent = QueueImpl->getLastEvent();
218-
if (QueueImpl->MDiscardEvents) {
219-
std::cout << "Discard event enabled" << std::endl;
220-
return LastEvent;
221-
}
222-
223-
auto LastEventImpl = detail::getSyclObjImpl(LastEvent);
224-
// If last event is default constructed event then we want to associate it
225-
// with the queue and record submission time if profiling is enabled. Such
226-
// event corresponds to NOP and its submit time is same as start time and
227-
// end time.
228-
if (!LastEventImpl->isContextInitialized()) {
229-
LastEventImpl->associateWithQueue(QueueImpl);
230-
LastEventImpl->setSubmissionTime();
231-
}
232-
return detail::createSyclObjFromImpl<event>(LastEventImpl);
217+
return QueueImpl->getLastEvent();
233218
}
234219

235220
/// Prevents any commands submitted afterward to this queue from executing
@@ -240,7 +225,7 @@ getBarrierEventForInorderQueueHelper(const detail::QueueImplPtr QueueImpl) {
240225
/// \return a SYCL event object, which corresponds to the queue the command
241226
/// group is being enqueued on.
242227
event queue::ext_oneapi_submit_barrier(const detail::code_location &CodeLoc) {
243-
if (is_in_order() && !impl->getCommandGraph())
228+
if (is_in_order() && !impl->getCommandGraph() && !impl->MIsProfilingEnabled)
244229
return getBarrierEventForInorderQueueHelper(impl);
245230

246231
return submit([=](handler &CGH) { CGH.ext_oneapi_barrier(); }, CodeLoc);
@@ -262,7 +247,8 @@ event queue::ext_oneapi_submit_barrier(const std::vector<event> &WaitList,
262247
auto EventImpl = detail::getSyclObjImpl(Event);
263248
return !EventImpl->isContextInitialized() || EventImpl->isNOP();
264249
});
265-
if (is_in_order() && !impl->getCommandGraph() && AllEventsEmptyOrNop)
250+
if (is_in_order() && !impl->getCommandGraph() && !impl->MIsProfilingEnabled &&
251+
AllEventsEmptyOrNop)
266252
return getBarrierEventForInorderQueueHelper(impl);
267253

268254
return submit([=](handler &CGH) { CGH.ext_oneapi_barrier(WaitList); },
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// RUN: %{build} -o %t.out
2+
// RUN: %{run} %t.out
3+
4+
//==----------------- in_order_barrier_profiling.cpp -----------------------==//
5+
//
6+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
7+
// See https://llvm.org/LICENSE.txt for license information.
8+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
9+
//
10+
//===----------------------------------------------------------------------===//
11+
// Level Zero adapter has a similar in-order queue barrier optimization that
12+
// leads to incorrect profiling values.
13+
// UNSUPPORTED: level_zero
14+
#include <sycl/detail/core.hpp>
15+
16+
#include <sycl/properties/all_properties.hpp>
17+
18+
using namespace sycl;
19+
20+
// Checks that the barrier profiling info is consistent with the previous
21+
// command, despite the fact that the latter started after the barrier was
22+
// submitted.
23+
int main() {
24+
queue Q({property::queue::in_order(), property::queue::enable_profiling()});
25+
26+
buffer<int, 1> Buf(range<1>(1));
27+
event KernelEvent;
28+
event BarrierEvent;
29+
{
30+
auto HostAcc = Buf.get_access();
31+
KernelEvent = Q.submit([&](handler &cgh) {
32+
auto Acc = Buf.get_access(cgh);
33+
cgh.single_task([=]() {});
34+
});
35+
BarrierEvent = Q.ext_oneapi_submit_barrier();
36+
}
37+
uint64_t KernelEnd =
38+
KernelEvent.get_profiling_info<info::event_profiling::command_end>();
39+
uint64_t BarrierStart =
40+
BarrierEvent.get_profiling_info<info::event_profiling::command_start>();
41+
assert(KernelEnd <= BarrierStart);
42+
}

sycl/test-e2e/Regression/nop_event_profiling.cpp

Lines changed: 0 additions & 38 deletions
This file was deleted.

0 commit comments

Comments
 (0)