Skip to content

Commit

Permalink
[SYCL][Graph] Skip Graph tests based on sycl-ls output (#12812)
Browse files Browse the repository at this point in the history
The graph extension tests are currently skipped during execution for
devices which don't support the graphs extension. However, this early
return causes the tests to be reported as passed and makes it hard from
looking at the results to know if the tests actually stressed the graphs
code or not.

Improved this situation by changing the SYCL-Graph device info query to
an aspect such that `sycl-ls --verbose` will output `ext_oneapi_graph`
for supported devices. This can then be used to inform the LIT config
and set a requirement for tests, enabling the tests to be obviously
skipped for devices that don't support graphs.

To enable setting this requirement in `lit.local.cfg` files some extra
directories have been created, in particular `UnsupportedDevice` which
contains tests that don't have a requirement as the tests verify
expected errors are thrown when using the graphs API with unsupported
devices.

The removal of the device info query means that we can no longer report
if a device emulates support for SYCL-Graph, however we currently have
no such implementations as they haven't yet deemed to provide enough
value. This is technically an ABI breaking change however due to the
removal of symbols, but SYCL-Graph is currently an experimental
extension so such changes may be permitted.
  • Loading branch information
EwanC authored Mar 11, 2024
1 parent 6b0066a commit a6301e9
Show file tree
Hide file tree
Showing 155 changed files with 173 additions and 674 deletions.
3 changes: 2 additions & 1 deletion llvm/include/llvm/SYCLLowerIR/DeviceConfigFile.td
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ def AspectExt_oneapi_tangle_group : Aspect<"ext_oneapi_tangle_group">;
def AspectExt_intel_matrix : Aspect<"ext_intel_matrix">;
def AspectExt_oneapi_is_composite : Aspect<"ext_oneapi_is_composite">;
def AspectExt_oneapi_is_component : Aspect<"ext_oneapi_is_component">;
def AspectExt_oneapi_graph : Aspect<"ext_oneapi_graph">;
// Deprecated aspects
def AspectInt64_base_atomics : Aspect<"int64_base_atomics">;
def AspectInt64_extended_atomics : Aspect<"int64_extended_atomics">;
Expand Down Expand Up @@ -119,7 +120,7 @@ def : TargetInfo<"__TestAspectList",
AspectExt_oneapi_interop_semaphore_import, AspectExt_oneapi_interop_semaphore_export,
AspectExt_oneapi_mipmap, AspectExt_oneapi_mipmap_anisotropy, AspectExt_oneapi_mipmap_level_reference, AspectExt_intel_esimd,
AspectExt_oneapi_ballot_group, AspectExt_oneapi_fixed_size_group, AspectExt_oneapi_opportunistic_group,
AspectExt_oneapi_tangle_group, AspectExt_intel_matrix, AspectExt_oneapi_is_composite, AspectExt_oneapi_is_component],
AspectExt_oneapi_tangle_group, AspectExt_intel_matrix, AspectExt_oneapi_is_composite, AspectExt_oneapi_is_component, AspectExt_oneapi_graph],
[]>;
// This definition serves the only purpose of testing whether the deprecated aspect list defined in here and in SYCL RT
// match.
Expand Down
34 changes: 8 additions & 26 deletions sycl/doc/extensions/experimental/sycl_ext_oneapi_graph.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Dan Holmes, Intel +
Greg Lueck, Intel +
Steffen Larsen, Intel +
Jaime Arteaga Molina, Intel +
Andrei Elovikov, Intel +
Ewan Crawford, Codeplay +
Ben Tracy, Codeplay +
Duncan McBain, Codeplay +
Expand Down Expand Up @@ -296,37 +297,18 @@ Adding an executable graph as a sub-graph does not affect its existing node
dependencies, such that it could be submitted in future without any side
effects of prior uses as a sub-graph.

=== Device Info Query

[source, c++]
----
namespace sycl::ext::oneapi::experimental {
enum class graph_support_level {
unsupported,
native,
emulated
};
}
----
=== Querying Device Support

Due to the experimental nature of the extension, support is not available across
all devices. The following device support query is added to the
`sycl::ext::oneapi::experimental` namespace for reporting devices which are
are currently supported, and how that support is implemented.
all devices.

Table {counter: tableNumber}. Device Info Queries.
Table {counter: tableNumber}. Device Support Aspect.
[%header]
|===
| Device Descriptors | Return Type | Description

|`info::device::graph_support`
|`graph_support_level`
|When passed to `device::get_info<...>()`, the function returns `native`
if there is an underlying SYCL backend command-buffer construct which is used
to propagate the graph to the backend. If no backend construct exists, or
building on top of it has not yet been implemented, then `emulated` is
returned. Otherwise `unsupported` is returned if the SYCL device doesn't
support using this graph extension.
| Device Descriptor | Description

|`aspect::ext_oneapi_graph`
| Indicates that the device supports the APIs described in this extension.

|===

Expand Down
10 changes: 10 additions & 0 deletions sycl/include/sycl/device_aspect_macros.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,11 @@
#define __SYCL_ALL_DEVICES_HAVE_ext_oneapi_is_component__ 0
#endif

#ifndef __SYCL_ALL_DEVICES_HAVE_ext_oneapi_graph__
// __SYCL_ASPECT(ext_oneapi_graph, 61)
#define __SYCL_ALL_DEVICES_HAVE_ext_oneapi_graph__ 0
#endif

#ifndef __SYCL_ANY_DEVICE_HAS_host__
// __SYCL_ASPECT(host, 0)
#define __SYCL_ANY_DEVICE_HAS_host__ 0
Expand Down Expand Up @@ -617,3 +622,8 @@
// __SYCL_ASPECT(ext_oneapi_is_component, 60)
#define __SYCL_ANY_DEVICE_HAS_ext_oneapi_is_component__ 0
#endif

#ifndef __SYCL_ANY_DEVICE_HAS_ext_oneapi_graph__
// __SYCL_ASPECT(ext_oneapi_graph, 61)
#define __SYCL_ANY_DEVICE_HAS_ext_oneapi_graph__ 0
#endif
1 change: 1 addition & 0 deletions sycl/include/sycl/info/aspects.def
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,4 @@ __SYCL_ASPECT(ext_oneapi_tangle_group, 57)
__SYCL_ASPECT(ext_intel_matrix, 58)
__SYCL_ASPECT(ext_oneapi_is_composite, 59)
__SYCL_ASPECT(ext_oneapi_is_component, 60)
__SYCL_ASPECT(ext_oneapi_graph, 61)
5 changes: 0 additions & 5 deletions sycl/include/sycl/info/ext_oneapi_device_traits.def
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@ __SYCL_PARAM_TRAITS_SPEC(ext::oneapi::experimental, device, matrix_combinations,
std::vector<ext::oneapi::experimental::matrix::combination>,
PI_EXT_ONEAPI_DEVICE_INFO_MATRIX_COMBINATIONS)

__SYCL_PARAM_TRAITS_SPEC(
ext::oneapi::experimental, device, graph_support,
ext::oneapi::experimental::graph_support_level,
0 /* No PI device code needed */)

// Bindless images pitched allocation
__SYCL_PARAM_TRAITS_SPEC(ext::oneapi::experimental, device,
image_row_pitch_align, uint32_t,
Expand Down
9 changes: 2 additions & 7 deletions sycl/include/sycl/info/info_desc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,14 +193,9 @@ template <typename T, T param> struct compatibility_param_traits {};
} /*namespace info */ \
} /*namespace Namespace */

namespace ext::oneapi::experimental {

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

namespace info::device {
namespace ext::oneapi::experimental::info::device {
template <int Dimensions> struct max_work_groups;
} // namespace info::device
} // namespace ext::oneapi::experimental
} // namespace ext::oneapi::experimental::info::device
#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
25 changes: 25 additions & 0 deletions sycl/source/detail/device_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,31 @@ bool device_impl::has(aspect Aspect) const {

return Result != nullptr;
}
case aspect::ext_oneapi_graph: {
size_t ResultSize = 0;
bool CallSuccessful = getPlugin()->call_nocheck<PiApiKind::piDeviceGetInfo>(
MDevice, PI_DEVICE_INFO_EXTENSIONS, 0, nullptr,
&ResultSize) == PI_SUCCESS;
if (!CallSuccessful || ResultSize == 0) {
return PI_FALSE;
}

std::unique_ptr<char[]> Result(new char[ResultSize]);
CallSuccessful = getPlugin()->call_nocheck<PiApiKind::piDeviceGetInfo>(
MDevice, PI_DEVICE_INFO_EXTENSIONS, ResultSize,
Result.get(), nullptr) == PI_SUCCESS;

if (!CallSuccessful) {
return PI_FALSE;
}

std::string_view ExtensionsString(Result.get());
std::cout << ExtensionsString;
const bool Support =
ExtensionsString.find("ur_exp_command_buffer") != std::string::npos;

return Support;
}
}
throw runtime_error("This device aspect has not been implemented yet.",
PI_ERROR_INVALID_DEVICE);
Expand Down
35 changes: 0 additions & 35 deletions sycl/source/detail/device_info.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1185,34 +1185,6 @@ struct get_device_info_impl<
}
};

// Specialization for graph extension support
template <>
struct get_device_info_impl<
ext::oneapi::experimental::graph_support_level,
ext::oneapi::experimental::info::device::graph_support> {
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::graph_support_level::unsupported;

std::unique_ptr<char[]> Result(new char[ResultSize]);
Dev->getPlugin()->call<PiApiKind::piDeviceGetInfo>(
Dev->getHandleRef(), PI_DEVICE_INFO_EXTENSIONS, ResultSize,
Result.get(), nullptr);

std::string_view ExtensionsString(Result.get());
bool CmdBufferSupport =
ExtensionsString.find("ur_exp_command_buffer") != std::string::npos;
return CmdBufferSupport
? ext::oneapi::experimental::graph_support_level::native
: ext::oneapi::experimental::graph_support_level::unsupported;
}
};

// Specialization for composite devices extension.
template <>
struct get_device_info_impl<
Expand Down Expand Up @@ -2175,13 +2147,6 @@ inline uint32_t get_device_info_host<
PI_ERROR_INVALID_DEVICE);
}

template <>
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::graph_support_level::unsupported;
}

template <>
inline uint32_t get_device_info_host<
ext::oneapi::experimental::info::device::image_row_pitch_align>() {
Expand Down
22 changes: 3 additions & 19 deletions sycl/source/detail/graph_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@
#include <sycl/feature_test.hpp>
#include <sycl/queue.hpp>

// Developer switch to use emulation mode on all backends, even those that
// report native support, this is useful for debugging.
#define FORCE_EMULATION_MODE 0

namespace sycl {
inline namespace _V1 {

Expand Down Expand Up @@ -1288,21 +1284,9 @@ void executable_command_graph::finalizeImpl() {
impl->makePartitions();

auto Device = impl->getGraphImpl()->getDevice();
bool CmdBufSupport =
Device
.get_info<ext::oneapi::experimental::info::device::graph_support>() ==
graph_support_level::native;

#if FORCE_EMULATION_MODE
// Above query should still succeed in emulation mode, but ignore the
// result and use emulation.
CmdBufSupport = false;
#endif
if (CmdBufSupport) {
for (auto Partition : impl->getPartitions()) {
if (!Partition->isHostTask()) {
impl->createCommandBuffers(Device, Partition);
}
for (auto Partition : impl->getPartitions()) {
if (!Partition->isHostTask()) {
impl->createCommandBuffers(Device, Partition);
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions sycl/source/detail/graph_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -586,9 +586,7 @@ class graph_impl {
MAllowBuffers = true;
}

if (SyclDevice.get_info<
ext::oneapi::experimental::info::device::graph_support>() ==
graph_support_level::unsupported) {
if (!SyclDevice.has(aspect::ext_oneapi_graph)) {
std::stringstream Stream;
Stream << SyclDevice.get_backend();
std::string BackendString = Stream.str();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// when trying to use sycl_ext_oneapi_device_global
// along with Graph.

#include "graph_common.hpp"
#include "../graph_common.hpp"

using TestProperties = decltype(sycl::ext::oneapi::experimental::properties{});

Expand Down Expand Up @@ -143,10 +143,6 @@ template <OperationPath PathKind> void test(queue Queue) {
int main() {
queue Queue;

if (!are_graphs_supported(Queue)) {
return 0;
}

test<OperationPath::Explicit>(Queue);
test<OperationPath::RecordReplay>(Queue);
test<OperationPath::Shortcut>(Queue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,11 @@
// Tests that calling handler::depends_on() for events not part of the graph
// throws.

#include "graph_common.hpp"
#include "../graph_common.hpp"

int main() {
queue Queue{};

if (!are_graphs_supported(Queue)) {
return 0;
}

ext::oneapi::experimental::command_graph Graph{Queue.get_context(),
Queue.get_device()};
ext::oneapi::experimental::command_graph Graph2{Queue.get_context(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,11 @@
// Tests that waiting on an event returned from a Record and Replay submission
// throws.

#include "graph_common.hpp"
#include "../graph_common.hpp"

int main() {
queue Queue{};

if (!are_graphs_supported(Queue)) {
return 0;
}

ext::oneapi::experimental::command_graph Graph{Queue.get_context(),
Queue.get_device()};
Graph.begin_recording(Queue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,11 @@

// Tests that waiting on a Queue in recording mode throws.

#include "graph_common.hpp"
#include "../graph_common.hpp"

int main() {
queue Queue{};

if (!are_graphs_supported(Queue)) {
return 0;
}

ext::oneapi::experimental::command_graph Graph{Queue.get_context(),
Queue.get_device()};
Graph.begin_recording(Queue);
Expand Down
1 change: 1 addition & 0 deletions sycl/test-e2e/Graph/Error/lit.local.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
config.required_features += ['aspect-ext_oneapi_graph']
4 changes: 0 additions & 4 deletions sycl/test-e2e/Graph/Explicit/add_node_while_recording.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@
int main() {
queue Queue{};

if (!are_graphs_supported(Queue)) {
return 0;
}

bool Success = false;

exp_ext::command_graph Graph{Queue.get_context(), Queue.get_device()};
Expand Down
3 changes: 2 additions & 1 deletion sycl/test-e2e/Graph/Explicit/basic_usm_host.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
// RUN: %if level_zero %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=0 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %}
// Extra run to check for immediate-command-list in Level Zero
// RUN: %if level_zero && linux %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=1 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %}
//

// REQUIRES: aspect-usm_host_allocations

#define GRAPH_E2E_EXPLICIT

Expand Down
4 changes: 3 additions & 1 deletion sycl/test-e2e/Graph/Explicit/basic_usm_mixed.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
// RUN: %if level_zero %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=0 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %}
// Extra run to check for immediate-command-list in Level Zero
// RUN: %if level_zero && linux %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=1 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %}
//

// REQUIRES: aspect-usm_host_allocations
// REQUIRES: aspect-usm_shared_allocations

#define GRAPH_E2E_EXPLICIT

Expand Down
3 changes: 2 additions & 1 deletion sycl/test-e2e/Graph/Explicit/basic_usm_shared.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
// RUN: %if level_zero %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=0 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %}
// Extra run to check for immediate-command-list in Level Zero
// RUN: %if level_zero && linux %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=1 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %}
//

// REQUIRES: aspect-usm_shared_allocations

#define GRAPH_E2E_EXPLICIT

Expand Down
3 changes: 2 additions & 1 deletion sycl/test-e2e/Graph/Explicit/basic_usm_system.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
// RUN: %if level_zero %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=0 %{l0_leak_check} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %}
// Extra run to check for immediate-command-list in Level Zero
// RUN: %if level_zero && linux %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=1 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %}
//

// REQUIRES: aspect-usm_system_allocations

#define GRAPH_E2E_EXPLICIT

Expand Down
Loading

0 comments on commit a6301e9

Please sign in to comment.