Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for SimSYCL as a SYCL implementation #871

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
cmake_minimum_required(VERSION 3.15)
project(sycl_cts LANGUAGES CXX)

set(CMAKE_CXX_STANDARD 17)
if(SYCL_IMPLEMENTATION STREQUAL SimSYCL)
set(CMAKE_CXX_STANDARD 20)
else()
set(CMAKE_CXX_STANDARD 17)
endif()

set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS ON) # Required for hex floats in C++11 mode on gcc 6+
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/bin")
Expand Down
2 changes: 1 addition & 1 deletion ci/generate_exclude_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def parse_arguments():
configuration-time test category filters for all failing targets.""")

parser.add_argument('sycl_implementation', metavar="SYCL-Implementation",
choices=['DPCPP', 'hipSYCL'], type=str,
choices=['DPCPP', 'hipSYCL', 'SimSYCL'], type=str,
help="The SYCL implementation to use")
parser.add_argument('--cmake-args', type=str,
help="Arguments to pass on to CMake during configuration")
Expand Down
20 changes: 20 additions & 0 deletions ci/simsycl.filter
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
accessor_legacy
atomic
atomic_ref_stress
buffer
device
exception_handling
handler
image
image_accessor
invoke
kernel
kernel_args
kernel_bundle
math_builtin_api
multi_ptr
queue
reduction
sampler
spec_constants
stream
45 changes: 45 additions & 0 deletions cmake/AdaptSimSYCL.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
add_library(SYCL::SYCL INTERFACE IMPORTED GLOBAL)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this file is named AdaptSimSYCL and not SimSYCL?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've had Find* and Adapt* files for a while now, it has nothing to do with AdaptiveCpp - the corresponding file for AdaptiveCpp in #874 is called AdaptAdaptiveCpp ;-).

target_link_libraries(SYCL::SYCL INTERFACE SimSYCL::simsycl)
# add_sycl_executable_implementation function
# Builds a SYCL program, compiling multiple SYCL test case source files into a
# test executable, invoking a single-source/device compiler
# Parameters are:
# - NAME Name of the test executable
# - OBJECT_LIBRARY Name of the object library of all the compiled test cases
# - TESTS List of SYCL test case source files to be built into the
# test executable
function(add_sycl_executable_implementation)
cmake_parse_arguments(args "" "NAME;OBJECT_LIBRARY" "TESTS" ${ARGN})
set(exe_name ${args_NAME})
set(object_lib_name ${args_OBJECT_LIBRARY})
set(test_cases_list ${args_TESTS})

add_library(${object_lib_name} OBJECT ${test_cases_list})
add_executable(${exe_name} $<TARGET_OBJECTS:${object_lib_name}>)

# hipSYCL needs the macro to be called on both the object library (to
Copy link
Contributor

@psalz psalz Feb 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# hipSYCL needs the macro to be called on both the object library (to
# SimSYCL needs the macro to be called on both the object library (to

Copy pasta - although I think this isn't even true?

# override the compiler) and the executable (to override the linker).
add_sycl_to_target(TARGET ${object_lib_name} SOURCES ${test_cases_list})
add_sycl_to_target(TARGET ${exe_name})

set_target_properties(${object_lib_name} PROPERTIES
INCLUDE_DIRECTORIES $<TARGET_PROPERTY:${exe_name},INCLUDE_DIRECTORIES>
COMPILE_DEFINITIONS $<TARGET_PROPERTY:${exe_name},COMPILE_DEFINITIONS>
COMPILE_OPTIONS $<TARGET_PROPERTY:${exe_name},COMPILE_OPTIONS>
COMPILE_FEATURES $<TARGET_PROPERTY:${exe_name},COMPILE_FEATURES>
POSITION_INDEPENDENT_CODE ON)
endfunction()

function(add_sycl_to_target)
set(options)
set(one_value_keywords TARGET)
set(multi_value_keywords SOURCES)
cmake_parse_arguments(ADD_SYCL
"${options}"
"${one_value_keywords}"
"${multi_value_keywords}"
${ARGN}
)

target_link_libraries(${ADD_SYCL_TARGET} PUBLIC SimSYCL::simsycl)
endfunction()
4 changes: 2 additions & 2 deletions cmake/AddSYCLExecutable.cmake
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
set (KNOWN_SYCL_IMPLEMENTATIONS "Intel_SYCL;DPCPP;hipSYCL")
set (KNOWN_SYCL_IMPLEMENTATIONS "Intel_SYCL;DPCPP;hipSYCL;SimSYCL")
if (NOT ${SYCL_IMPLEMENTATION} IN_LIST KNOWN_SYCL_IMPLEMENTATIONS)
message(FATAL_ERROR
"The SYCL CTS requires specifying a SYCL implementation with "
"-DSYCL_IMPLEMENTATION=[Intel_SYCL,DPCPP;hipSYCL]")
"-DSYCL_IMPLEMENTATION=[Intel_SYCL,DPCPP;hipSYCL;SimSYCL]")
endif()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should just put KNOWN_SYCL_IMPLEMENTATIONS directly into the message instead of modifying it every time we change list of known implementations?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps CTS_GRADE_SYCL_IMPLEMENTATION or something better, because we know quite many other implementations. :-)


if(NOT TARGET OpenCL_Proxy)
Expand Down
4 changes: 2 additions & 2 deletions tests/common/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -413,8 +413,8 @@ namespace pixel_tag {
struct upper: generic {};
};

// hipSYCL does not yet support images
#if !SYCL_CTS_COMPILING_WITH_HIPSYCL
// hipSYCL and SimSYCL do not yet support images
#if !SYCL_CTS_COMPILING_WITH_HIPSYCL && !SYCL_CTS_COMPILING_WITH_SIMSYCL

/**
* @brief Helps with retrieving the right access type for reading/writing
Expand Down
2 changes: 2 additions & 0 deletions tests/common/disabled_for_test_case.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@
#define INTERNAL_CTS_SYCL_IMPL_DPCPP ()
#elif SYCL_CTS_COMPILING_WITH_HIPSYCL
#define INTERNAL_CTS_SYCL_IMPL_hipSYCL ()
#elif SYCL_CTS_COMPILING_WITH_SIMSYCL
#define INTERNAL_CTS_SYCL_IMPL_SimSYCL ()
#else
#error Unknown SYCL implementation
#endif
Expand Down
16 changes: 16 additions & 0 deletions tests/event/event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ TEST_CASE("event::get_backend returns the associated backend", "[event]") {

TEST_CASE("event::get_wait_list returns a list of all direct dependencies",
"[event]") {
#if SYCL_CTS_COMPILING_WITH_SIMSYCL
SKIP("SimSYCL does not implement asynchronous execution.");
#endif
Comment on lines +77 to +79
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that from the conformance point of view the better to use DISABLED_FOR_TEST_CASE macro so the test appears as failed.


resolvable_host_event e_a;
resolvable_host_event e_b{{e_a.get_sycl_event()}};
resolvable_host_event e_c;
Expand Down Expand Up @@ -117,6 +121,10 @@ class delayed_host_event : public resolvable_host_event {
};

TEST_CASE("event can be waited upon", "[event]") {
#if SYCL_CTS_COMPILING_WITH_SIMSYCL
SKIP("SimSYCL does not implement asynchronous execution.");
#endif

// Give main thread some time to fail the did_resolve check
delayed_host_event dhe{std::chrono::milliseconds(100)};

Expand All @@ -138,6 +146,10 @@ TEST_CASE("event can be waited upon", "[event]") {
}

TEST_CASE("multiple events can be waited upon simultaneously", "[event]") {
#if SYCL_CTS_COMPILING_WITH_SIMSYCL
SKIP("SimSYCL does not implement asynchronous execution.");
#endif

// Give main thread some time to fail the did_resolve check
delayed_host_event dhe1{std::chrono::milliseconds(100)};
delayed_host_event dhe2{std::chrono::milliseconds(100)};
Expand Down Expand Up @@ -302,6 +314,10 @@ TEST_CASE("event::get_info returns correct command execution status",
sycl::info::event_command_status>(make_device_event());

SECTION("for host_task event") {
#if SYCL_CTS_COMPILING_WITH_SIMSYCL
SKIP("SimSYCL does not implement asynchronous execution.");
#endif

resolvable_host_event rhe;
auto& event = rhe.get_sycl_event();

Expand Down
4 changes: 4 additions & 0 deletions tests/event/event_semantics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ TEST_CASE("event common reference semantics", "[event]") {
}

TEST_CASE("event common reference semantics, mutation", "[event]") {
#if SYCL_CTS_COMPILING_WITH_SIMSYCL
SKIP("SimSYCL does not implement asynchronous execution.");
#endif

resolvable_host_event dependent_event;
resolvable_host_event rhe_t0{{dependent_event.get_sycl_event()}};

Expand Down
6 changes: 6 additions & 0 deletions tests/usm/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
file(GLOB test_cases_list *.cpp)

if(SYCL_IMPLEMENTATION STREQUAL SimSYCL)
message(WARNING "SimSYCL does not provide true concurrency between host and device, disabling USM atomic tests")
list(FILTER test_cases_list EXCLUDE REGEX usm_atomic_access_.*\\.cpp$)
endif()
Comment on lines +3 to +6
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Piggybacking off of @AlexeySachkov's comment, these should preferably also be disabled using DISABLE_FOR_TEST_CASE(SimSYCL, ...!



add_cts_test(${test_cases_list})
9 changes: 9 additions & 0 deletions util/sycl_exceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@
#define SYCL_CTS_SUPPORT_HAS_EXCEPTION_CATEGORY 1
#define SYCL_CTS_SUPPORT_HAS_MAKE_ERROR_CODE 0

#elif SYCL_CTS_COMPILING_WITH_SIMSYCL
// Feature flags for SimSYCL

#define SYCL_CTS_SUPPORT_HAS_EXCEPTION_CODE 1
#define SYCL_CTS_SUPPORT_HAS_EXCEPTION_CATEGORY 1
#define SYCL_CTS_SUPPORT_HAS_ERRC_FOR 0
#define SYCL_CTS_SUPPORT_HAS_ERROR_CATEGORY_FOR 0
#define SYCL_CTS_SUPPORT_HAS_MAKE_ERROR_CODE 1

#else

#define SYCL_CTS_SUPPORT_HAS_EXCEPTION_CODE 1
Expand Down
Loading