Convert id tests to Catch2, update for SYCL 2020
#246
Conversation
|
(Looks like I forgot to add |
There was a problem hiding this comment.
Interesting.
I wonder whether it would not me to have more functional programming to have kernel fusion to have all the DEVICE_EVAL contents to be evaluated in a few kernels instead of 1 kernel per DEVICE_EVAL. I am just thinking to the kernel compilation time on FPGA... ;-)
tests/common/device_eval.h
Outdated
| */ | ||
| #define DEVICE_EVAL(expr) DEVICE_EVAL_T(decltype(expr), expr) | ||
|
|
||
| #endif // __SYCLCTS_TESTS_COMMON_DEVICE_EVAL_H |
There was a problem hiding this comment.
I've added an .editorconfig file with the appropriate setting.
There was a problem hiding this comment.
Interesting https://editorconfig.org/
I didn't know about it.
| * - Operands must exist in surrounding scope ([=] capture). | ||
| * - No lambda expressions (requires C++20). Use DEVICE_EVAL_T instead. | ||
| */ | ||
| #define DEVICE_EVAL(expr) DEVICE_EVAL_T(decltype(expr), expr) |
There was a problem hiding this comment.
Why using macros DEVICE_EVAL_T or DEVICE_EVAL at the first place instead of functions or lambdas?
There was a problem hiding this comment.
Not sure what you mean, how would I pass an unevaluated expression into a function/lambda?
tests/common/device_eval.h
Outdated
| #include <sycl/sycl.hpp> | ||
|
|
||
| #define DEVICE_EVAL_T(T, expr) \ | ||
| ([=]() { \ |
There was a problem hiding this comment.
| ([=]() { \ | |
| ([=] { \ |
tests/common/device_eval.h
Outdated
| sycl_cts::util::get_cts_object::queue() \ | ||
| .submit([=, &result_buf](sycl::handler& cgh) { \ | ||
| sycl::accessor result{result_buf, cgh, sycl::write_only}; \ | ||
| cgh.single_task([=]() { result[0] = expr; }); \ |
There was a problem hiding this comment.
| cgh.single_task([=]() { result[0] = expr; }); \ | |
| cgh.single_task([=] { result[0] = expr; }); \ |
tests/common/string_makers.h
Outdated
| }; | ||
| } // namespace Catch | ||
|
|
||
| #endif // __SYCLCTS_TESTS_COMMON_STRING_MAKERS_H |
tests/id/id.cpp
Outdated
| CHECK(DEVICE_EVAL(a[i]) == values[i]); | ||
| } | ||
|
|
||
| #define ASSIGN_COMPONENT(operand_value, c, v) \ |
There was a problem hiding this comment.
Because once you work with macros too much, everything seems like an opportunity for a macro ;-). Fixed.
| CHECK(DEVICE_EVAL(b >= a) == idh<D>::get(0, 0, 1)); | ||
| } | ||
|
|
||
| #define COMPOUND_OP(operand_value, expr) \ |
There was a problem hiding this comment.
Same as for DEVICE_EVAL, I want to pass an unevaluated expr into here.
Configured to use UNIX line endings for all files and add final newline.
|
I've addressed some of the feedback and added a note to the common by-value semantics test case that there is some uncertainty around its definition (cf KhronosGroup/SYCL-Docs#210). Also I need to take a look at @vasilytric's |
This is a comprehensive overhaul of the tests for the
sycl::idclass, using the new Catch2 testing framework idioms. The tests should now cover everything prescribed by the SYCL 2020 Revision 4 specification.I'm trying out a new pattern here, where instead of performing a bunch of tests on a SYCL device and recording each "assertion's" result in a buffer that is then bulk-asserted on the host, I'm offloading individual expressions to the device using the
DEVICE_EVALmacro. This, in my opinion, makes for overall much more readable test cases.It currently only compiles for DPC++ because: