Skip to content

Commit

Permalink
[SYCL][Graph] Implement missing exceptions defined by SYCL-Graphs spe…
Browse files Browse the repository at this point in the history 
…cification (intel#10775)

This PR contains a set of changes that implement throwing an exception
on invalid usage, as defined by
[sycl_ext_oneapi_graph](https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/proposed/sycl_ext_oneapi_graph.asciidoc).
Covering the cases missing from the current implementation.

* Error checking for `make_edge` API. 
* Throw exception when explicit add called on a graph recording a queue.
* Throw exception when creating a graph for an unsupported backend.
* Error on invalid buffer behaviour when used with graphs to reflect
intel#10473

## Authors

Co-authored-by: Pablo Reble <pablo.reble@intel.com>
Co-authored-by: Julian Miller <julian.miller@intel.com>
Co-authored-by: Ben Tracy <ben.tracy@codeplay.com>
Co-authored-by: Ewan Crawford <ewan@codeplay.com>
Co-authored-by: Maxime France-Pillois
<maxime.francepillois@codeplay.com>
  • Loading branch information
@EwanC
EwanC authored Aug 30, 2023
1 parent e1163ff commit 77b794b
Show file tree
Hide file tree
Showing 37 changed files with 1,112 additions and 350 deletions.
25 changes: 25 additions & 0 deletions sycl/include/sycl/accessor.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,7 @@ class __SYCL_EXPORT AccessorBaseHost {
const range<3> &getMemoryRange() const;
void *getPtr() const noexcept;
bool isPlaceholder() const;
bool isMemoryObjectUsedByGraph() const;

detail::AccHostDataT &getAccData();

Expand Down Expand Up @@ -1487,6 +1488,18 @@ class __SYCL_EBO __SYCL_SPECIAL_CLASS __SYCL_TYPE(accessor) accessor :
typename std::iterator_traits<iterator>::difference_type;
using size_type = std::size_t;

/// If creating a host_accessor this checks to see if the underlying memory
/// object is currently in use by a command_graph, and throws if it is.
void throwIfUsedByGraph() const {
#ifndef __SYCL_DEVICE_ONLY__
if (IsHostBuf && AccessorBaseHost::isMemoryObjectUsedByGraph()) {
throw sycl::exception(make_error_code(errc::invalid),
"Host accessors cannot be created for buffers "
"which are currently in use by a command graph.");
}
#endif
}

// The list of accessor constructors with their arguments
// -------+---------+-------+----+-----+--------------
// Dimensions = 0
Expand Down Expand Up @@ -1566,6 +1579,7 @@ class __SYCL_EBO __SYCL_SPECIAL_CLASS __SYCL_TYPE(accessor) accessor :
detail::getSyclObjImpl(BufferRef).get(), AdjustedDim, sizeof(DataT),
IsPlaceH, BufferRef.OffsetInBytes, BufferRef.IsSubBuffer,
PropertyList) {
throwIfUsedByGraph();
preScreenAccessor(PropertyList);
if (!AccessorBaseHost::isPlaceholder())
addHostAccessorAndWait(AccessorBaseHost::impl.get());
Expand Down Expand Up @@ -1605,6 +1619,7 @@ class __SYCL_EBO __SYCL_SPECIAL_CLASS __SYCL_TYPE(accessor) accessor :
detail::getSyclObjImpl(BufferRef).get(), AdjustedDim, sizeof(DataT),
IsPlaceH, BufferRef.OffsetInBytes, BufferRef.IsSubBuffer,
PropertyList) {
throwIfUsedByGraph();
preScreenAccessor(PropertyList);
if (!AccessorBaseHost::isPlaceholder())
addHostAccessorAndWait(AccessorBaseHost::impl.get());
Expand Down Expand Up @@ -1640,6 +1655,7 @@ class __SYCL_EBO __SYCL_SPECIAL_CLASS __SYCL_TYPE(accessor) accessor :
getAdjustedMode(PropertyList),
detail::getSyclObjImpl(BufferRef).get(), Dimensions, sizeof(DataT),
BufferRef.OffsetInBytes, BufferRef.IsSubBuffer, PropertyList) {
throwIfUsedByGraph();
preScreenAccessor(PropertyList);
detail::associateWithHandler(CommandGroupHandler, this, AccessTarget);
initHostAcc();
Expand Down Expand Up @@ -1676,6 +1692,7 @@ class __SYCL_EBO __SYCL_SPECIAL_CLASS __SYCL_TYPE(accessor) accessor :
getAdjustedMode(PropertyList),
detail::getSyclObjImpl(BufferRef).get(), Dimensions, sizeof(DataT),
BufferRef.OffsetInBytes, BufferRef.IsSubBuffer, PropertyList) {
throwIfUsedByGraph();
preScreenAccessor(PropertyList);
detail::associateWithHandler(CommandGroupHandler, this, AccessTarget);
initHostAcc();
Expand Down Expand Up @@ -1708,6 +1725,7 @@ class __SYCL_EBO __SYCL_SPECIAL_CLASS __SYCL_TYPE(accessor) accessor :
detail::getSyclObjImpl(BufferRef).get(), Dimensions, sizeof(DataT),
IsPlaceH, BufferRef.OffsetInBytes, BufferRef.IsSubBuffer,
PropertyList) {
throwIfUsedByGraph();
preScreenAccessor(PropertyList);
if (!AccessorBaseHost::isPlaceholder())
addHostAccessorAndWait(AccessorBaseHost::impl.get());
Expand Down Expand Up @@ -1743,6 +1761,7 @@ class __SYCL_EBO __SYCL_SPECIAL_CLASS __SYCL_TYPE(accessor) accessor :
detail::getSyclObjImpl(BufferRef).get(), Dimensions, sizeof(DataT),
IsPlaceH, BufferRef.OffsetInBytes, BufferRef.IsSubBuffer,
PropertyList) {
throwIfUsedByGraph();
preScreenAccessor(PropertyList);
if (!AccessorBaseHost::isPlaceholder())
addHostAccessorAndWait(AccessorBaseHost::impl.get());
Expand Down Expand Up @@ -1805,6 +1824,7 @@ class __SYCL_EBO __SYCL_SPECIAL_CLASS __SYCL_TYPE(accessor) accessor :
getAdjustedMode(PropertyList),
detail::getSyclObjImpl(BufferRef).get(), Dimensions, sizeof(DataT),
BufferRef.OffsetInBytes, BufferRef.IsSubBuffer, PropertyList) {
throwIfUsedByGraph();
preScreenAccessor(PropertyList);
detail::associateWithHandler(CommandGroupHandler, this, AccessTarget);
initHostAcc();
Expand Down Expand Up @@ -1839,6 +1859,7 @@ class __SYCL_EBO __SYCL_SPECIAL_CLASS __SYCL_TYPE(accessor) accessor :
getAdjustedMode(PropertyList),
detail::getSyclObjImpl(BufferRef).get(), Dimensions, sizeof(DataT),
BufferRef.OffsetInBytes, BufferRef.IsSubBuffer, PropertyList) {
throwIfUsedByGraph();
preScreenAccessor(PropertyList);
initHostAcc();
detail::associateWithHandler(CommandGroupHandler, this, AccessTarget);
Expand Down Expand Up @@ -2014,6 +2035,7 @@ class __SYCL_EBO __SYCL_SPECIAL_CLASS __SYCL_TYPE(accessor) accessor :
detail::getSyclObjImpl(BufferRef).get(), Dimensions,
sizeof(DataT), IsPlaceH, BufferRef.OffsetInBytes,
BufferRef.IsSubBuffer, PropertyList) {
throwIfUsedByGraph();
preScreenAccessor(PropertyList);
if (!AccessorBaseHost::isPlaceholder())
addHostAccessorAndWait(AccessorBaseHost::impl.get());
Expand Down Expand Up @@ -2056,6 +2078,7 @@ class __SYCL_EBO __SYCL_SPECIAL_CLASS __SYCL_TYPE(accessor) accessor :
detail::getSyclObjImpl(BufferRef).get(), Dimensions,
sizeof(DataT), IsPlaceH, BufferRef.OffsetInBytes,
BufferRef.IsSubBuffer, PropertyList) {
throwIfUsedByGraph();
preScreenAccessor(PropertyList);
if (!AccessorBaseHost::isPlaceholder())
addHostAccessorAndWait(AccessorBaseHost::impl.get());
Expand Down Expand Up @@ -2127,6 +2150,7 @@ class __SYCL_EBO __SYCL_SPECIAL_CLASS __SYCL_TYPE(accessor) accessor :
detail::getSyclObjImpl(BufferRef).get(), Dimensions,
sizeof(DataT), BufferRef.OffsetInBytes,
BufferRef.IsSubBuffer, PropertyList) {
throwIfUsedByGraph();
preScreenAccessor(PropertyList);
if (BufferRef.isOutOfBounds(AccessOffset, AccessRange,
BufferRef.get_range()))
Expand Down Expand Up @@ -2169,6 +2193,7 @@ class __SYCL_EBO __SYCL_SPECIAL_CLASS __SYCL_TYPE(accessor) accessor :
detail::getSyclObjImpl(BufferRef).get(), Dimensions,
sizeof(DataT), BufferRef.OffsetInBytes,
BufferRef.IsSubBuffer, PropertyList) {
throwIfUsedByGraph();
preScreenAccessor(PropertyList);
if (BufferRef.isOutOfBounds(AccessOffset, AccessRange,
BufferRef.get_range()))
Expand Down
4 changes: 3 additions & 1 deletion sycl/include/sycl/detail/property_helper.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ enum DataLessPropKind {
GraphNoCycleCheck = 19,
QueueSubmissionBatched = 20,
QueueSubmissionImmediate = 21,
GraphAssumeDataOutlivesBuffer = 22,
GraphAssumeBufferOutlivesGraph = 23,
// Indicates the last known dataless property.
LastKnownDataLessPropKind = 21,
LastKnownDataLessPropKind = 23,
// Exceeding 32 may cause ABI breaking change on some of OSes.
DataLessPropKindSize = 32
};
Expand Down
11 changes: 10 additions & 1 deletion sycl/include/sycl/ext/oneapi/experimental/graph.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,22 @@ namespace graph {

/// Property passed to command_graph constructor to disable checking for cycles.
///
/// \todo Cycle check not yet implemented.
class no_cycle_check : public ::sycl::detail::DataLessProperty<
::sycl::detail::GraphNoCycleCheck> {
public:
no_cycle_check() = default;
};

/// Property passed to command_graph constructor to allow buffers to be used
/// with graphs. Passing this property represents a promise from the user that
/// the buffer will outlive any graph that it is used in.
///
class assume_buffer_outlives_graph
: public ::sycl::detail::DataLessProperty<
::sycl::detail::GraphAssumeBufferOutlivesGraph> {
public:
assume_buffer_outlives_graph() = default;
};
} // namespace graph

namespace node {
Expand Down
2 changes: 1 addition & 1 deletion sycl/include/sycl/info/ext_oneapi_device_traits.def
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ __SYCL_PARAM_TRAITS_SPEC(ext::oneapi::experimental, device, architecture,
PI_EXT_ONEAPI_DEVICE_INFO_IP_VERSION)
__SYCL_PARAM_TRAITS_SPEC(
ext::oneapi::experimental, device, graph_support,
ext::oneapi::experimental::info::graph_support_level,
ext::oneapi::experimental::graph_support_level,
0 /* No PI device code needed */)

// Bindless images pitched allocation
Expand Down
10 changes: 5 additions & 5 deletions sycl/include/sycl/info/info_desc.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,14 +191,14 @@ template <typename T, T param> struct compatibility_param_traits {};
} /*namespace info */ \
} /*namespace Namespace */

namespace ext::oneapi::experimental::info {
namespace ext::oneapi::experimental {

enum class graph_support_level { unsupported = 0, native, emulated };
enum class graph_support_level { unsupported = 0, native = 1, emulated = 2 };

namespace device {
namespace info::device {
template <int Dimensions> struct max_work_groups;
} // namespace device
} // namespace ext::oneapi::experimental::info
} // namespace info::device
} // namespace ext::oneapi::experimental
#include <sycl/info/ext_codeplay_device_traits.def>
#include <sycl/info/ext_intel_device_traits.def>
#include <sycl/info/ext_oneapi_device_traits.def>
Expand Down
5 changes: 5 additions & 0 deletions sycl/source/accessor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//

#include <detail/queue_impl.hpp>
#include <detail/sycl_mem_obj_t.hpp>
#include <sycl/accessor.hpp>

namespace sycl {
Expand Down Expand Up @@ -94,6 +95,10 @@ void *AccessorBaseHost::getMemoryObject() const { return impl->MSYCLMemObj; }

bool AccessorBaseHost::isPlaceholder() const { return impl->MIsPlaceH; }

bool AccessorBaseHost::isMemoryObjectUsedByGraph() const {
return static_cast<detail::SYCLMemObjT *>(impl->MSYCLMemObj)->isUsedInGraph();
}

LocalAccessorBaseHost::LocalAccessorBaseHost(
sycl::range<3> Size, int Dims, int ElemSize,
const property_list &PropertyList) {
Expand Down
15 changes: 7 additions & 8 deletions sycl/source/detail/device_info.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -934,16 +934,16 @@ struct get_device_info_impl<
// Specialization for graph extension support
template <>
struct get_device_info_impl<
ext::oneapi::experimental::info::graph_support_level,
ext::oneapi::experimental::graph_support_level,
ext::oneapi::experimental::info::device::graph_support> {
static ext::oneapi::experimental::info::graph_support_level
static ext::oneapi::experimental::graph_support_level
get(const DeviceImplPtr &Dev) {
size_t ResultSize = 0;
Dev->getPlugin()->call<PiApiKind::piDeviceGetInfo>(
Dev->getHandleRef(), PI_DEVICE_INFO_EXTENSIONS, 0, nullptr,
&ResultSize);
if (ResultSize == 0)
return ext::oneapi::experimental::info::graph_support_level::unsupported;
return ext::oneapi::experimental::graph_support_level::unsupported;

std::unique_ptr<char[]> Result(new char[ResultSize]);
Dev->getPlugin()->call<PiApiKind::piDeviceGetInfo>(
Expand All @@ -954,9 +954,8 @@ struct get_device_info_impl<
bool CmdBufferSupport =
ExtensionsString.find("ur_exp_command_buffer") != std::string::npos;
return CmdBufferSupport
? ext::oneapi::experimental::info::graph_support_level::native
: ext::oneapi::experimental::info::graph_support_level::
unsupported;
? ext::oneapi::experimental::graph_support_level::native
: ext::oneapi::experimental::graph_support_level::unsupported;
}
};

Expand Down Expand Up @@ -1862,10 +1861,10 @@ inline uint32_t get_device_info_host<
}

template <>
inline ext::oneapi::experimental::info::graph_support_level
inline ext::oneapi::experimental::graph_support_level
get_device_info_host<ext::oneapi::experimental::info::device::graph_support>() {
// No support for graphs on the host device.
return ext::oneapi::experimental::info::graph_support_level::unsupported;
return ext::oneapi::experimental::graph_support_level::unsupported;
}

template <>
Expand Down
Loading

0 comments on commit 77b794b

Please sign in to comment.