Repository: llvm/llvm-project
File: libsycl/src/detail/context_impl.hpp (C++)

Code before:
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef _LIBSYCL_CONTEXT_IMPL
#define _LIBSYCL_CONTEXT_IMPL

#include <sycl/__impl/context.hpp>
#include <sycl/__impl/detail/config.hpp>

#include <OffloadAPI.h>

#include <functional>

_LIBSYCL_BEGIN_NAMESPACE_SYCL
namespace detail {

class PlatformImpl;
class DeviceImpl;

/// Context dummy (w/o liboffload handle) that represents all devices
/// in platform.
///
/// Presence of context object is essential for many APIs. This dummy is a way
/// to support them in case of absence of context support in liboffload. For
/// backends where context exists and participates in operations liboffload
/// plugins create and use default context that represents all devices in that
/// platform. Duplicating this logic here.
class ContextImpl : public std::enable_shared_from_this<ContextImpl> {
  struct Private {
    explicit Private() = default;
  };

public:
  /// Constructs a ContextImpl using a platform.
  ///
  /// Newly created instance represents all devices in platform.
  ///
  /// \param Platform is a platform to associate this context with.
  ContextImpl(PlatformImpl &Platform, Private) : MPlatform(Platform) {}

  /// Constructs a ContextImpl with a provided arguments. Variadic helper.
  /// Restrics ways of ContextImpl creation.
  template <typename... Ts>
  static std::shared_ptr<ContextImpl> create(Ts &&...args) {
    return std::make_shared<ContextImpl>(std::forward<Ts>(args)..., Private{});
  }

Review comment (suggestion):
Please, move the part about unfinished implementation to a separate TODO comment.

"represents all devices in platform" is misleading definition of the SYCL context. I suggest using definition from the [SYCL spec glossary](https://registry.khronos.org/SYCL/specs/sycl-2020/html/sycl-2020.html#context):

> Context represents the runtime data structures and state required by a SYCL backend API to interact with a group of devices associated with a platform.


Code after:
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef _LIBSYCL_CONTEXT_IMPL
#define _LIBSYCL_CONTEXT_IMPL

#include <sycl/__impl/context.hpp>
#include <sycl/__impl/detail/config.hpp>

#include <OffloadAPI.h>

#include <functional>

_LIBSYCL_BEGIN_NAMESPACE_SYCL
namespace detail {

class PlatformImpl;
class DeviceImpl;

// TODO: Presence of context object is essential for many APIs. Current
// implementation of this class is a way to support them in case of absence of
// context support in liboffload. For backends where context exists and
// participates in operations, liboffload plugins create and use default context
// that represents all devices in that platform. Duplicating this logic here.
/// Context represents the runtime data structures and state required by a SYCL
/// backend API to interact with a group of devices associated with a platform.
class ContextImpl : public std::enable_shared_from_this<ContextImpl> {
  struct Private {
    explicit Private() = default;
  };

public:
  /// Constructs a ContextImpl using a platform.
  ///
  /// Newly created instance represents all devices in platform.
  ///
  /// \param Platform is a platform to associate this context with.
  ContextImpl(PlatformImpl &Platform, Private) : MPlatform(Platform) {}

  /// Constructs a ContextImpl with a provided arguments. Variadic helper.
  /// Restrics ways of ContextImpl creation.
  template <typename... Ts>
  static std::shared_ptr<ContextImpl> create(Ts &&...args) {
    return std::make_shared<ContextImpl>(std::forward<Ts>(args)..., Private{});
  }