Test for `[[sycl::reqd_work_group_size]]` exception when mismatched `nd_range` was given #285

andreyromanof · 2022-02-16T01:46:57Z

This PR provides test for optional kernel features.
Test decorates kernel with attribute [[sycl::reqd_work_group_size(N)]] where N is supported size and pass nd_range instance with unsupported size to parallel_for invocation and expect exception sycl::errc::nd_range.

Depend on:

Disable _Pragma to work around Clang 8 bug #296

…nd_range` was given

ProGTX

Some concerns about disabling the test for implementations that don't support these features.

ProGTX · 2022-02-17T09:05:51Z

ci/computecpp.filter

@@ -28,3 +28,4 @@ usm
 vector_api
 vector_constructors
 vector_swizzles
+optional_kernel_features


Do you maybe have output from a run of ComputeCpp or is this just to play it safe and not risk breaking CI?

Here output from a run of ComputeCpp before I apply filter. On line 2791 warning messages about unknown attributes, which is why I decided to put test in the filter.

But right now I also noticed other errors like no known conversion from 'cl::sycl::nd_item<1>' to 'sycl::item<1>' on line 2814. Am I wrong with the lambda argument? Do I have to use nd_item<1> instead of item<1> on line 41 in kernel_features_mismatched_nd_range_exception.cpp?

ProGTX · 2022-02-17T09:08:39Z

ci/hipsycl.filter

@@ -37,3 +37,4 @@ vector_load_store
 vector_operators
 vector_swizzle_assignment
 vector_swizzles
+optional_kernel_features


I think it's better to maintain an alphabetical ordering, the same for other filters. Also, should we maybe name it kernel_optional_features?

Sorted lines in this commit: a02a67f.

We can rename it but in other PR when all tests will be implemented, because some of the tests in other PR, some of the tests in development and it will be unhandy to rename these tests now.

IMHO, I think we can keep the current name.

ProGTX · 2022-02-17T09:12:08Z

tests/optional_kernel_features/kernel_features_mismatched_nd_range_exception.cpp

+  }
+
+  const bool is_exception_expected = true;
+  sycl::errc errc_expected = sycl::errc::nd_range;


From the spec:

In order to guarantee source code portability of SYCL applications that use optional kernel features, all SYCL implementations must be able to compile device code that uses these optional features regardless of whether the implementation supports the features on any of its devices.

You've marked two implementations in the CI as not passing (adding the test to the filter), but I believe they should be able to pass the test even if they don't implement the features, otherwise they wouldn't be able to pass conformance in the current CTS design.

As I understand CI doesn't run tests, it's just built it. Here you can find output from the building of tests on CI. ComputeCpp and hipsycl failed building due to some errors.

From hipsycl logs, line 1970: error: ‘errc’ is not a member of ‘sycl’. Looks like enum with error codes is not implemented yet and attributes too (line 1966).

Similar situation for computecpp.

But maybe I was wrong when I put these two implementations in the filter because both of them give the error message: no known conversion from 'cl::sycl::nd_item<1>' to 'sycl::item<1>' error, so it can be bug of tests itself.

It was my mistake. I changed to nd_item but now getting another error. Right now investigating it.

Return optional features to filter for hipsycl

andreyromanof · 2022-02-18T08:29:11Z

I tried to fix the code for the computecpp CI but looks like this implementation has a few specific issues:

It's doesn't support implementation-defined kernel names according to paragraph 4.9.4.2. Link to the error
I changed const auto max_wg_size to const size_t max_wg_size, because otherwise compilation was failed. This variable is used in the constructor of nd_range Link to error
Problem with linkage I was unable to solve, but my guess that its kind of stringify function for the sycl::errc that cause an error.

tests/optional_kernel_features/kernel_features_mismatched_nd_range_exception.cpp

Changed `testing_wg_size` to 1 that supports by all devices Changed `INFO` to `SECTION`

keryell

Thanks.

…into optional_kernel_features/missmatched_nd_range_exception

Removed optional_kernel_features from ci filter

Removed section due to issue in CI build

This reverts commit 12d9404.

andreyromanof · 2022-02-28T05:34:59Z

Build error on ComputeCpp points to SECTION invocation and use of undeclared identifier INTERNAL_CTS_ENABLED_TEST_CASE_BODY. Don't have any more ideas on how to fix it and seems like it is not a compiler bug, so we can't just add the test to filter.

psalz · 2022-02-28T09:04:24Z

Build error on ComputeCpp points to SECTION invocation and use of undeclared identifier INTERNAL_CTS_ENABLED_TEST_CASE_BODY. Don't have any more ideas on how to fix it and seems like it is not a compiler bug, so we can't just add the test to filter.

Actually the root cause is the error before that: error: _Pragma takes a parenthesized string literal. I've actually seen this before (it's a bug in Clang 8) and already have a fix for it. I'll open a PR shortly. This will however not really solve the problem at hand, as ComputeCpp then still fails during linking with undefined reference to cl::sycl::detail::errc_to_str[abi:cxx11](cl::sycl::detail::errc) (cc @ProGTX).

psalz · 2022-02-28T11:42:55Z

The aforementioned bug should be fixed by #296.

…into optional_kernel_features/missmatched_nd_range_exception

bader · 2022-03-10T07:46:28Z

@andreyromanof, please, fix the build with computecpp.

…into optional_kernel_features/missmatched_nd_range_exception

bader · 2022-03-10T08:44:55Z

@ProGTX, please, check if your concerns are addressed.

Update compile-time properties tests after spec update

Test for [[sycl::reqd_work_group_size]] exception when mismatched `…

3822a72

…nd_range` was given

andreyromanof requested review from ProGTX, bader, illuhad and keryell as code owners February 16, 2022 01:46

Romanov AndreyX added 3 commits February 16, 2022 10:52

Add optional_kernel_features to filter for computecpp and hipsycl

b03925a

Fix typo in file name

d50c118

Fix typo in code

ee28a82

bader requested a review from psalz February 16, 2022 07:17

ProGTX requested changes Feb 17, 2022

View reviewed changes

ProGTX mentioned this pull request Feb 17, 2022

Tests for [[device_has]] attribute from optional kernel features #287

Merged

Romanov AndreyX added 4 commits February 18, 2022 09:39

Sort lines

a02a67f

Change item to nd_item in kernels args

88bda24

Add kernel names

373ae90

Return optional features to filter for hipsycl

Replace auto to size_to for max_wg_size

9871be2

Return computecpp to the filter

bb33e39

andreyromanof requested a review from ProGTX February 18, 2022 08:33

psalz requested changes Feb 21, 2022

View reviewed changes

Apply comments

36a27b4

Changed `testing_wg_size` to 1 that supports by all devices Changed `INFO` to `SECTION`

bader requested a review from psalz February 22, 2022 10:08

psalz approved these changes Feb 22, 2022

View reviewed changes

Remove duplicate of the test tag from the test name

78147fe

keryell approved these changes Feb 25, 2022

View reviewed changes

Romanov AndreyX added 4 commits February 28, 2022 09:44

Merge branch 'SYCL-2020' of https://github.com/KhronosGroup/SYCL-CTS …

e09a86b

…into optional_kernel_features/missmatched_nd_range_exception

Add compile time disable for hipSYCL

8d399e3

Removed optional_kernel_features from ci filter

Temorarly remove of SECTION

12d9404

Removed section due to issue in CI build

Revert "Temorarly remove of SECTION"

9bb4f1a

This reverts commit 12d9404.

keryell approved these changes Mar 1, 2022

View reviewed changes

Merge branch 'SYCL-2020' of https://github.com/KhronosGroup/SYCL-CTS …

329bbbb

…into optional_kernel_features/missmatched_nd_range_exception

Romanov AndreyX added 2 commits March 10, 2022 14:54

Merge branch 'SYCL-2020' of https://github.com/KhronosGroup/SYCL-CTS …

b790797

…into optional_kernel_features/missmatched_nd_range_exception

Disable mismathched nd range test for ComputeCpp

951f4ee

ProGTX approved these changes Mar 10, 2022

View reviewed changes

bader merged commit e7ff3a1 into KhronosGroup:SYCL-2020 Mar 10, 2022

andreyromanof deleted the optional_kernel_features/missmatched_nd_range_exception branch April 7, 2022 08:02

steffenlarsen pushed a commit to steffenlarsen/SYCL-CTS that referenced this pull request Dec 6, 2022

Merge pull request KhronosGroup#285 from kopylovanat/refactor-properties

339f95e

Update compile-time properties tests after spec update

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test for `[[sycl::reqd_work_group_size]]` exception when mismatched `nd_range` was given #285

Test for `[[sycl::reqd_work_group_size]]` exception when mismatched `nd_range` was given #285

andreyromanof commented Feb 16, 2022 •

edited

ProGTX left a comment

ProGTX Feb 17, 2022

andreyromanof Feb 17, 2022

ProGTX Feb 17, 2022

andreyromanof Feb 17, 2022 •

edited

ProGTX Feb 17, 2022

andreyromanof Feb 17, 2022

andreyromanof Feb 18, 2022

andreyromanof commented Feb 18, 2022

keryell left a comment

andreyromanof commented Feb 28, 2022

psalz commented Feb 28, 2022 •

edited

psalz commented Feb 28, 2022

bader commented Mar 10, 2022

bader commented Mar 10, 2022

Test for [[sycl::reqd_work_group_size]] exception when mismatched nd_range was given #285

Test for [[sycl::reqd_work_group_size]] exception when mismatched nd_range was given #285

Conversation

andreyromanof commented Feb 16, 2022 • edited

ProGTX left a comment

Choose a reason for hiding this comment

ProGTX Feb 17, 2022

Choose a reason for hiding this comment

andreyromanof Feb 17, 2022

Choose a reason for hiding this comment

ProGTX Feb 17, 2022

Choose a reason for hiding this comment

andreyromanof Feb 17, 2022 • edited

Choose a reason for hiding this comment

ProGTX Feb 17, 2022

Choose a reason for hiding this comment

andreyromanof Feb 17, 2022

Choose a reason for hiding this comment

andreyromanof Feb 18, 2022

Choose a reason for hiding this comment

andreyromanof commented Feb 18, 2022

keryell left a comment

Choose a reason for hiding this comment

andreyromanof commented Feb 28, 2022

psalz commented Feb 28, 2022 • edited

psalz commented Feb 28, 2022

bader commented Mar 10, 2022

bader commented Mar 10, 2022

Test for `[[sycl::reqd_work_group_size]]` exception when mismatched `nd_range` was given #285

Test for `[[sycl::reqd_work_group_size]]` exception when mismatched `nd_range` was given #285

andreyromanof commented Feb 16, 2022 •

edited

andreyromanof Feb 17, 2022 •

edited

psalz commented Feb 28, 2022 •

edited