[NFCI][SYCL] Copy handler::MQueue to handler_impl (#18830)

Ideally, it should be just moved there, but doing the whole move in one
PR would be harder to review as it would require multiple internal API
changes to accept `queue_impl` by raw ptr/ref instead of
`std::shared_ptr<queue_impl>` value/ref. As such, this PR does the
following:

* Store `std::variant` of queue/graph instead of just graph as two are
mutually exclusive
* Store them by reference (through `std::reference_wrapper` as
'std::variant' can't store raw references directly) instead of
`std::shared_ptr<graph_impl>`. The reason for that is that the object
`handler` is submitted to is guaranteed to be alive for the whole
lifetime of the `handler` and we don't need to extend the lifetime of
queue/graph. Corresponding changes for `handler::MQueue` have been
implemented recently already (although via `std::shared_ptr<queue_impl>
&` and not raw reference) but are limited to prevew-only. Do the same
for graphs here, essentially.
* Update all uses of the old `handler_impl::MGraph` data member as it
needs to go through new getters accessing the `std::variant` described
above.
* Update some of the direct usages of `handler::MQueue` that don't
require APIs refactoring elsewhere. The remaining uses are left to the
subsequent PR(s).
* We'll probably need to keep the `handler::MQueue` initialized properly
even after the move is complete and all internal SYCL RT accesses are
through `handler_impl` as some direct `handler::MQueue` accesses might
have been inlined into the users' applications (I'd be especially
worried about reductions).

Author: aelovikov-intel

sycl/source/accessor.cpp

@@ -14,15 +14,6 @@
 namespace sycl {
 inline namespace _V1 {
 namespace detail {
-device getDeviceFromHandler(handler &cgh) {
-  assert((cgh.MQueue || getSyclObjImpl(cgh)->MGraph) &&
-         "One of MQueue or MGraph should be nonnull!");
-  if (cgh.MQueue)
-    return cgh.MQueue->get_device();
-
-  return getSyclObjImpl(cgh)->MGraph->getDevice();
-}
-
 // property::no_init is supported now for
 // accessor
 // host_accessor

sycl/source/detail/context_impl.hpp

@@ -29,7 +29,7 @@ inline namespace _V1 {
 // Forward declaration
 class device;
 namespace detail {
-class context_impl : std::enable_shared_from_this<context_impl> {
+class context_impl : public std::enable_shared_from_this<context_impl> {
   struct private_tag {
     explicit private_tag() = default;
   };

sycl/source/detail/graph_impl.cpp

@@ -504,7 +504,7 @@ graph_impl::add(std::function<void(handler &)> CGF,
                 std::vector<std::shared_ptr<node_impl>> &Deps) {
   (void)Args;
 #ifdef __INTEL_PREVIEW_BREAKING_CHANGES
-  detail::handler_impl HandlerImpl{shared_from_this()};
+  detail::handler_impl HandlerImpl{*this};
   sycl::handler Handler{&HandlerImpl, std::shared_ptr<detail::queue_impl>{}};
 #else
   sycl::handler Handler{shared_from_this()};
@@ -2284,7 +2284,7 @@ void dynamic_command_group_impl::finalizeCGFList(
     // Handler defined inside the loop so it doesn't appear to the runtime
     // as a single command-group with multiple commands inside.
 #ifdef __INTEL_PREVIEW_BREAKING_CHANGES
-    detail::handler_impl HandlerImpl{MGraph};
+    detail::handler_impl HandlerImpl{*MGraph};
     sycl::handler Handler{&HandlerImpl, std::shared_ptr<detail::queue_impl>{}};
 #else
     sycl::handler Handler{MGraph};

sycl/source/detail/handler_impl.hpp

@@ -10,6 +10,7 @@
 
 #include "sycl/handler.hpp"
 #include <detail/cg.hpp>
+#include <detail/graph_impl.hpp>
 #include <detail/kernel_bundle_impl.hpp>
 #include <memory>
 #include <sycl/ext/oneapi/experimental/graph.hpp>
@@ -31,15 +32,13 @@ enum class HandlerSubmissionState : std::uint8_t {
 
 class handler_impl {
 public:
-  handler_impl(queue_impl *SubmissionSecondaryQueue, bool EventNeeded)
+  handler_impl(queue_impl &Queue, queue_impl *SubmissionSecondaryQueue,
+               bool EventNeeded)
       : MSubmissionSecondaryQueue(SubmissionSecondaryQueue),
-        MEventNeeded(EventNeeded) {};
+        MEventNeeded(EventNeeded), MQueueOrGraph{Queue} {};
 
-  handler_impl(
-      std::shared_ptr<ext::oneapi::experimental::detail::graph_impl> Graph)
-      : MGraph{Graph} {}
-
-  handler_impl() = default;
+  handler_impl(ext::oneapi::experimental::detail::graph_impl &Graph)
+      : MQueueOrGraph{Graph} {}
 
   void setStateExplicitKernelBundle() {
     if (MSubmissionState == HandlerSubmissionState::SPEC_CONST_SET_STATE)
@@ -165,8 +164,46 @@ class handler_impl {
   /// manipulations with version are required
   detail::CGType MCGType = detail::CGType::None;
 
-  /// The graph that is associated with this handler.
-  std::shared_ptr<ext::oneapi::experimental::detail::graph_impl> MGraph;
+  // This handler is associated with either a queue or a graph.
+  using graph_impl = ext::oneapi::experimental::detail::graph_impl;
+  const std::variant<std::reference_wrapper<queue_impl>,
+                     std::reference_wrapper<graph_impl>>
+      MQueueOrGraph;
+
+  queue_impl *get_queue_or_null() {
+    auto *Queue =
+        std::get_if<std::reference_wrapper<queue_impl>>(&MQueueOrGraph);
+    return Queue ? &Queue->get() : nullptr;
+  }
+  queue_impl &get_queue() {
+    return std::get<std::reference_wrapper<queue_impl>>(MQueueOrGraph).get();
+  }
+  graph_impl *get_graph_or_null() {
+    auto *Graph =
+        std::get_if<std::reference_wrapper<graph_impl>>(&MQueueOrGraph);
+    return Graph ? &Graph->get() : nullptr;
+  }
+  graph_impl &get_graph() {
+    return std::get<std::reference_wrapper<graph_impl>>(MQueueOrGraph).get();
+  }
+
+  // Make the following methods templates to avoid circular dependencies for the
+  // includes.
+  template <typename Self = handler_impl> detail::device_impl &get_device() {
+    Self *self = this;
+    if (auto *Queue = self->get_queue_or_null())
+      return Queue->getDeviceImpl();
+    else
+      return self->get_graph().getDeviceImpl();
+  }
+  template <typename Self = handler_impl> context_impl &get_context() {
+    Self *self = this;
+    if (auto *Queue = self->get_queue_or_null())
+      return *Queue->getContextImplPtr();
+    else
+      return *self->get_graph().getContextImplPtr();
+  }
+
   /// If we are submitting a graph using ext_oneapi_graph this will be the graph
   /// to be executed.
   std::shared_ptr<ext::oneapi::experimental::detail::exec_graph_impl>

sycl/source/detail/queue_impl.cpp

@@ -310,7 +310,7 @@ queue_impl::submit_impl(const detail::type_erased_cgfo_ty &CGF,
                         const detail::code_location &Loc, bool IsTopCodeLoc,
                         const v1::SubmissionInfo &SubmitInfo) {
 #ifdef __INTEL_PREVIEW_BREAKING_CHANGES
-  detail::handler_impl HandlerImplVal(SecondaryQueue, CallerNeedsEvent);
+  detail::handler_impl HandlerImplVal(*this, SecondaryQueue, CallerNeedsEvent);
   detail::handler_impl *HandlerImpl = &HandlerImplVal;
   // Inlining `Self` results in a crash when SYCL RT is built using MSVC with
   // optimizations enabled. No crash if built using OneAPI.