Skip to content

Commit 09a1495

Browse files
[NFCI][SYCL] Prefer raw ptr/ref for queue_impl
Continuation of the refactoring in #18715 #18748
1 parent 9457ac2 commit 09a1495

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+395
-399
lines changed

sycl/source/detail/cg.hpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -725,14 +725,11 @@ class CGHostTask : public CG {
725725
std::shared_ptr<detail::context_impl> MContext;
726726
std::vector<ArgDesc> MArgs;
727727

728-
CGHostTask(std::shared_ptr<HostTask> HostTask,
729-
std::shared_ptr<detail::queue_impl> Queue,
728+
// TODO: ref?
729+
CGHostTask(std::shared_ptr<HostTask> HostTask, detail::queue_impl *Queue,
730730
std::shared_ptr<detail::context_impl> Context,
731731
std::vector<ArgDesc> Args, CG::StorageInitHelper CGData,
732-
CGType Type, detail::code_location loc = {})
733-
: CG(Type, std::move(CGData), std::move(loc)),
734-
MHostTask(std::move(HostTask)), MQueue(Queue), MContext(Context),
735-
MArgs(std::move(Args)) {}
732+
CGType Type, detail::code_location loc = {});
736733
};
737734

738735
} // namespace detail

sycl/source/detail/event_impl.cpp

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -159,23 +159,24 @@ event_impl::event_impl(ur_event_handle_t Event, const context &SyclContext)
159159
}
160160
}
161161

162-
event_impl::event_impl(const QueueImplPtr &Queue)
163-
: MQueue{Queue}, MIsProfilingEnabled{!Queue || Queue->MIsProfilingEnabled} {
164-
if (Queue)
165-
this->setContextImpl(Queue->getContextImplPtr());
166-
else {
167-
MState.store(HES_NotComplete);
168-
MHostProfilingInfo.reset(new HostProfilingInfo());
169-
if (!MHostProfilingInfo)
170-
throw sycl::exception(
171-
sycl::make_error_code(sycl::errc::runtime),
172-
"Out of host memory " +
173-
codeToString(UR_RESULT_ERROR_OUT_OF_HOST_MEMORY));
174-
return;
175-
}
162+
event_impl::event_impl(queue_impl &Queue)
163+
: MQueue{Queue.weak_from_this()},
164+
MIsProfilingEnabled{Queue.MIsProfilingEnabled} {
165+
this->setContextImpl(Queue.getContextImplPtr());
176166
MState.store(HES_Complete);
177167
}
178168

169+
// TODO: comment about https://github.com/intel/llvm/pull/14370
170+
event_impl::event_impl(std::nullptr_t) : MQueue{}, MIsProfilingEnabled{true} {
171+
MState.store(HES_NotComplete);
172+
MHostProfilingInfo.reset(new HostProfilingInfo());
173+
if (!MHostProfilingInfo)
174+
throw sycl::exception(sycl::make_error_code(sycl::errc::runtime),
175+
"Out of host memory " +
176+
codeToString(UR_RESULT_ERROR_OUT_OF_HOST_MEMORY));
177+
return;
178+
}
179+
179180
void event_impl::setQueue(queue_impl &Queue) {
180181
MQueue = Queue.shared_from_this();
181182
MIsProfilingEnabled = Queue.MIsProfilingEnabled;
@@ -212,7 +213,7 @@ void *event_impl::instrumentationProlog(std::string &Name, int32_t StreamID,
212213
// queue is available with the wait events. We check to see if the
213214
// TraceEvent is available in the Queue object.
214215
void *TraceEvent = nullptr;
215-
if (QueueImplPtr Queue = MQueue.lock()) {
216+
if (auto Queue = MQueue.lock()) {
216217
TraceEvent = Queue->getTraceEvent();
217218
WaitEvent =
218219
(TraceEvent ? static_cast<xpti_td *>(TraceEvent) : GSYCLGraphEvent);
@@ -281,7 +282,7 @@ void event_impl::wait_and_throw(
281282
std::shared_ptr<sycl::detail::event_impl> Self) {
282283
wait(Self);
283284

284-
if (QueueImplPtr SubmittedQueue = MSubmittedQueue.lock())
285+
if (auto SubmittedQueue = MSubmittedQueue.lock())
285286
SubmittedQueue->throw_asynchronous();
286287
}
287288

@@ -426,7 +427,7 @@ event_impl::get_backend_info<info::platform::version>() const {
426427
"the info::platform::version info descriptor can "
427428
"only be queried with an OpenCL backend");
428429
}
429-
if (QueueImplPtr Queue = MQueue.lock()) {
430+
if (auto Queue = MQueue.lock()) {
430431
return Queue->getDeviceImpl()
431432
.get_platform()
432433
.get_info<info::platform::version>();
@@ -449,7 +450,7 @@ event_impl::get_backend_info<info::device::version>() const {
449450
"the info::device::version info descriptor can only "
450451
"be queried with an OpenCL backend");
451452
}
452-
if (QueueImplPtr Queue = MQueue.lock()) {
453+
if (auto Queue = MQueue.lock()) {
453454
return Queue->getDeviceImpl().get_info<info::device::version>();
454455
}
455456
return ""; // If the queue has been released, no device will be associated so
@@ -516,21 +517,21 @@ std::vector<EventImplPtr> event_impl::getWaitList() {
516517
return Result;
517518
}
518519

519-
void event_impl::flushIfNeeded(const QueueImplPtr &UserQueue) {
520+
void event_impl::flushIfNeeded(queue_impl *UserQueue) {
520521
// Some events might not have a native handle underneath even at this point,
521522
// e.g. those produced by memset with 0 size (no UR call is made).
522523
auto Handle = this->getHandle();
523524
if (MIsFlushed || !Handle)
524525
return;
525526

526-
QueueImplPtr Queue = MQueue.lock();
527+
auto Queue = MQueue.lock();
527528
// If the queue has been released, all of the commands have already been
528529
// implicitly flushed by urQueueRelease.
529530
if (!Queue) {
530531
MIsFlushed = true;
531532
return;
532533
}
533-
if (Queue == UserQueue)
534+
if (Queue.get() == UserQueue)
534535
return;
535536

536537
// Check if the task for this event has already been submitted.
@@ -568,7 +569,7 @@ void event_impl::setSubmissionTime() {
568569
if (!MIsProfilingEnabled && !MProfilingTagEvent)
569570
return;
570571

571-
if (QueueImplPtr Queue = MQueue.lock()) {
572+
if (auto Queue = MQueue.lock()) {
572573
try {
573574
MSubmitTime = Queue->getDeviceImpl().getCurrentDeviceTime();
574575
} catch (sycl::exception &e) {

sycl/source/detail/event_impl.hpp

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ class Adapter;
3131
class context_impl;
3232
using ContextImplPtr = std::shared_ptr<sycl::detail::context_impl>;
3333
class queue_impl;
34-
using QueueImplPtr = std::shared_ptr<sycl::detail::queue_impl>;
3534
class event_impl;
3635
using EventImplPtr = std::shared_ptr<sycl::detail::event_impl>;
3736

@@ -48,7 +47,7 @@ class event_impl {
4847
/// If the constructed SYCL event is waited on it will complete immediately.
4948
/// Normally constructs a host event, use std::nullopt to instead instantiate
5049
/// a device event.
51-
event_impl(std::optional<HostEventState> State = HES_Complete)
50+
event_impl(std::optional<HostEventState> State)
5251
: MIsFlushed(true), MState(State.value_or(HES_Complete)),
5352
MIsDefaultConstructed(!State), MIsHostEvent(State) {
5453
// Need to fail in event() constructor if there are problems with the
@@ -66,7 +65,9 @@ class event_impl {
6665
/// \param Event is a valid instance of UR event.
6766
/// \param SyclContext is an instance of SYCL context.
6867
event_impl(ur_event_handle_t Event, const context &SyclContext);
69-
event_impl(const QueueImplPtr &Queue);
68+
event_impl(queue_impl &Queue);
69+
// TODO: what about the very first ctor overload?
70+
event_impl(std::nullptr_t);
7071

7172
/// Sets a queue associated with the event
7273
///
@@ -209,7 +210,7 @@ class event_impl {
209210
/// Performs a flush on the queue associated with this event if the user queue
210211
/// is different and the task associated with this event hasn't been submitted
211212
/// to the device yet.
212-
void flushIfNeeded(const QueueImplPtr &UserQueue);
213+
void flushIfNeeded(queue_impl *UserQueue);
213214

214215
/// Cleans dependencies of this event_impl.
215216
void cleanupDependencyEvents();
@@ -229,7 +230,9 @@ class event_impl {
229230
///
230231
/// @return shared_ptr to MWorkerQueue, please be aware it can be empty
231232
/// pointer
232-
QueueImplPtr getWorkerQueue() { return MWorkerQueue.lock(); };
233+
std::shared_ptr<sycl::detail::queue_impl> getWorkerQueue() {
234+
return MWorkerQueue.lock();
235+
};
233236

234237
/// Sets worker queue for command.
235238
///
@@ -258,7 +261,9 @@ class event_impl {
258261
/// @return Submission time for command associated with this event
259262
uint64_t getSubmissionTime();
260263

261-
QueueImplPtr getSubmittedQueue() const { return MSubmittedQueue.lock(); };
264+
std::shared_ptr<sycl::detail::queue_impl> getSubmittedQueue() const {
265+
return MSubmittedQueue.lock();
266+
};
262267

263268
/// Checks if this event is complete.
264269
///

sycl/source/detail/graph_impl.cpp

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,9 @@ std::shared_ptr<node_impl> graph_impl::addNodesToExits(
404404
// Add all the new nodes to the node storage
405405
for (auto &Node : NodeList) {
406406
MNodeStorage.push_back(Node);
407-
addEventForNode(std::make_shared<sycl::detail::event_impl>(), Node);
407+
addEventForNode(std::make_shared<sycl::detail::event_impl>(
408+
sycl::detail::event_impl::HES_Complete),
409+
Node);
408410
}
409411

410412
return this->add(Outputs);
@@ -494,7 +496,9 @@ graph_impl::add(std::vector<std::shared_ptr<node_impl>> &Deps) {
494496

495497
addDepsToNode(NodeImpl, Deps);
496498
// Add an event associated with this explicit node for mixed usage
497-
addEventForNode(std::make_shared<sycl::detail::event_impl>(), NodeImpl);
499+
addEventForNode(std::make_shared<sycl::detail::event_impl>(
500+
sycl::detail::event_impl::HES_Complete),
501+
NodeImpl);
498502
return NodeImpl;
499503
}
500504

@@ -552,7 +556,9 @@ graph_impl::add(std::function<void(handler &)> CGF,
552556
this->add(NodeType, std::move(Handler.impl->MGraphNodeCG), Deps);
553557

554558
// Add an event associated with this explicit node for mixed usage
555-
addEventForNode(std::make_shared<sycl::detail::event_impl>(), NodeImpl);
559+
addEventForNode(std::make_shared<sycl::detail::event_impl>(
560+
sycl::detail::event_impl::HES_Complete),
561+
NodeImpl);
556562

557563
// Retrieve any dynamic parameters which have been registered in the CGF and
558564
// register the actual nodes with them.
@@ -651,7 +657,9 @@ graph_impl::add(std::shared_ptr<dynamic_command_group_impl> &DynCGImpl,
651657
add(NodeType, ActiveKernel, Deps);
652658

653659
// Add an event associated with this explicit node for mixed usage
654-
addEventForNode(std::make_shared<sycl::detail::event_impl>(), NodeImpl);
660+
addEventForNode(std::make_shared<sycl::detail::event_impl>(
661+
sycl::detail::event_impl::HES_Complete),
662+
NodeImpl);
655663

656664
// Track the dynamic command-group used inside the node object
657665
DynCGImpl->MNodes.push_back(NodeImpl);
@@ -897,7 +905,7 @@ exec_graph_impl::enqueueNode(ur_exp_command_buffer_handle_t CommandBuffer,
897905

898906
sycl::detail::EventImplPtr Event =
899907
sycl::detail::Scheduler::getInstance().addCG(
900-
Node->getCGCopy(), MQueueImpl,
908+
Node->getCGCopy(), *MQueueImpl,
901909
/*EventNeeded=*/true, CommandBuffer, Deps);
902910

903911
if (MIsUpdatable) {
@@ -1037,8 +1045,7 @@ exec_graph_impl::enqueue(sycl::detail::queue_impl &Queue,
10371045
PartitionsExecutionEvents;
10381046

10391047
auto CreateNewEvent([&]() {
1040-
auto NewEvent =
1041-
std::make_shared<sycl::detail::event_impl>(Queue.shared_from_this());
1048+
auto NewEvent = std::make_shared<sycl::detail::event_impl>(Queue);
10421049
NewEvent->setContextImpl(Queue.getContextImplPtr());
10431050
NewEvent->setStateIncomplete();
10441051
return NewEvent;
@@ -1122,7 +1129,7 @@ exec_graph_impl::enqueue(sycl::detail::queue_impl &Queue,
11221129
CommandBuffer, nullptr, std::move(CGData));
11231130

11241131
NewEvent = sycl::detail::Scheduler::getInstance().addCG(
1125-
std::move(CommandGroup), Queue.shared_from_this(),
1132+
std::move(CommandGroup), Queue,
11261133
/*EventNeeded=*/true);
11271134
}
11281135
NewEvent->setEventFromSubmittedExecCommandBuffer(true);
@@ -1142,7 +1149,7 @@ exec_graph_impl::enqueue(sycl::detail::queue_impl &Queue,
11421149
.MQueue = Queue.shared_from_this();
11431150

11441151
NewEvent = sycl::detail::Scheduler::getInstance().addCG(
1145-
NodeImpl->getCGCopy(), Queue.shared_from_this(),
1152+
NodeImpl->getCGCopy(), Queue,
11461153
/*EventNeeded=*/true);
11471154
}
11481155
PartitionsExecutionEvents[CurrentPartition] = NewEvent;
@@ -1424,7 +1431,7 @@ void exec_graph_impl::update(
14241431
// other scheduler commands
14251432
auto UpdateEvent =
14261433
sycl::detail::Scheduler::getInstance().addCommandGraphUpdate(
1427-
this, Nodes, MQueueImpl, std::move(UpdateRequirements),
1434+
this, Nodes, MQueueImpl.get(), std::move(UpdateRequirements),
14281435
MExecutionEvents);
14291436

14301437
MExecutionEvents.push_back(UpdateEvent);

sycl/source/detail/graph_impl.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ class node_impl : public std::enable_shared_from_this<node_impl> {
288288

289289
return std::make_unique<sycl::detail::CGHostTask>(
290290
sycl::detail::CGHostTask(
291-
std::move(HostTaskSPtr), CommandGroupPtr->MQueue,
291+
std::move(HostTaskSPtr), CommandGroupPtr->MQueue.get(),
292292
CommandGroupPtr->MContext, std::move(NewArgs), std::move(Data),
293293
CommandGroupPtr->getType(), Loc));
294294
}

sycl/source/detail/helpers.hpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ class event;
2121
namespace detail {
2222
class CGExecKernel;
2323
class queue_impl;
24-
using QueueImplPtr = std::shared_ptr<sycl::detail::queue_impl>;
2524
class RTDeviceBinaryImage;
2625

2726
#ifdef __INTEL_PREVIEW_BREAKING_CHANGES

sycl/source/detail/memory_manager.hpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ class queue_impl;
2828
class event_impl;
2929
class context_impl;
3030

31-
using QueueImplPtr = std::shared_ptr<detail::queue_impl>;
3231
using EventImplPtr = std::shared_ptr<detail::event_impl>;
3332
using ContextImplPtr = std::shared_ptr<detail::context_impl>;
3433

sycl/source/detail/queue_impl.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,10 @@ queue_impl::get_backend_info<info::device::backend_version>() const {
114114
}
115115
#endif
116116

117-
static event prepareSYCLEventAssociatedWithQueue(
118-
const std::shared_ptr<detail::queue_impl> &QueueImpl) {
117+
static event
118+
prepareSYCLEventAssociatedWithQueue(detail::queue_impl &QueueImpl) {
119119
auto EventImpl = std::make_shared<detail::event_impl>(QueueImpl);
120-
EventImpl->setContextImpl(detail::getSyclObjImpl(QueueImpl->get_context()));
120+
EventImpl->setContextImpl(detail::getSyclObjImpl(QueueImpl.get_context()));
121121
EventImpl->setStateIncomplete();
122122
return detail::createSyclObjFromImpl<event>(EventImpl);
123123
}
@@ -462,7 +462,7 @@ event queue_impl::submitMemOpHelper(const std::vector<event> &DepEvents,
462462
return createDiscardedEvent();
463463
}
464464

465-
event ResEvent = prepareSYCLEventAssociatedWithQueue(shared_from_this());
465+
event ResEvent = prepareSYCLEventAssociatedWithQueue(*this);
466466
const auto &EventImpl = detail::getSyclObjImpl(ResEvent);
467467
{
468468
NestedCallsTracker tracker;

sycl/source/detail/queue_impl.hpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -648,9 +648,12 @@ class queue_impl : public std::enable_shared_from_this<queue_impl> {
648648
// for in order ones.
649649
void revisitUnenqueuedCommandsState(const EventImplPtr &CompletedHostTask);
650650

651-
static ContextImplPtr getContext(const QueueImplPtr &Queue) {
651+
static ContextImplPtr getContext(queue_impl *Queue) {
652652
return Queue ? Queue->getContextImplPtr() : nullptr;
653653
}
654+
static ContextImplPtr getContext(const std::shared_ptr<queue_impl> &Queue) {
655+
return getContext(Queue.get());
656+
}
654657

655658
// Must be called under MMutex protection
656659
void doUnenqueuedCommandCleanup(
@@ -663,7 +666,7 @@ class queue_impl : public std::enable_shared_from_this<queue_impl> {
663666
/// will wait for the completion of all work in the queue at the time of the
664667
/// insertion, but will not act as a barrier unless the queue is in-order.
665668
EventImplPtr insertMarkerEvent() {
666-
auto ResEvent = std::make_shared<detail::event_impl>(shared_from_this());
669+
auto ResEvent = std::make_shared<detail::event_impl>(*this);
667670
ur_event_handle_t UREvent = nullptr;
668671
getAdapter()->call<UrApiKind::urEnqueueEventsWait>(getHandleRef(), 0,
669672
nullptr, &UREvent);
@@ -687,9 +690,8 @@ class queue_impl : public std::enable_shared_from_this<queue_impl> {
687690
protected:
688691
template <typename HandlerType = handler>
689692
EventImplPtr insertHelperBarrier(const HandlerType &Handler) {
690-
auto &Queue = Handler.impl->get_queue();
691-
auto ResEvent =
692-
std::make_shared<detail::event_impl>(Queue.shared_from_this());
693+
queue_impl &Queue = Handler.impl->get_queue();
694+
auto ResEvent = std::make_shared<detail::event_impl>(Queue);
693695
ur_event_handle_t UREvent = nullptr;
694696
getAdapter()->call<UrApiKind::urEnqueueEventsWaitWithBarrier>(
695697
Queue.getHandleRef(), 0, nullptr, &UREvent);

sycl/source/detail/reduction.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ __SYCL_EXPORT size_t reduGetPreferredWGSize(std::shared_ptr<queue_impl> &Queue,
207207
__SYCL_EXPORT void
208208
addCounterInit(handler &CGH, std::shared_ptr<sycl::detail::queue_impl> &Queue,
209209
std::shared_ptr<int> &Counter) {
210-
auto EventImpl = std::make_shared<detail::event_impl>(Queue);
210+
auto EventImpl = std::make_shared<detail::event_impl>(*Queue);
211211
EventImpl->setContextImpl(detail::getSyclObjImpl(Queue->get_context()));
212212
EventImpl->setStateIncomplete();
213213
ur_event_handle_t UREvent = nullptr;

0 commit comments

Comments
 (0)