[SYCL] Fix uncaught exceptions and null dereference

[bso-intel]

Added proper handling of exceptions propagated to the outermost level.

[bso-intel]

@dm-vodopyanov
I think I addressed all your feedback.

[bso-intel]

@dm-vodopyanov
gentle ping

[bso-intel]

@intel/llvm-gatekeepers
Please merge.

[bso-intel]

@intel/llvm-gatekeepers
Please merge.

[AlexeySachkov]

Changes like this look weird to me and I think that we may need some spec changes here to avoid them. Tagging @gmlueck here for awareness/feedback.

The only meaningful exception I would expect to see from range::size() is something like overflow_error, but at the same time we expect this type to be used on device (as a return type of various nd_item/group-related APIs) and exceptions are not supported in there.

Changing definition of the method between host and device compilation seems hacky, so I assume that the SYCL spec can only have one definition. And I think that that definition should be noexcept.

Reason behind our implementation throwing from range::size():

• range inherits from detail::common_array to store data
• range::size uses detail::common_array::operator[] to access that data
• detail::common_array::operator[] contains a call to check_dimensions which throws if index is out of range [0..2], but does that only on host

I would also explore if we should turn runtime check in detail::common_array::check_dimensions into an assert. Even though values we pass there could pass directly from user (via id::operator[] for example), the SYCL spec is not clear what happens if we use an invalid dimension here. KhronosGroup/SYCL-Docs#551 is related, but there is no conclusion.

[gmlueck]

The change here seems unnecessary. Even without this change, if an exception is thrown in the body of size, the application will terminate. That's the semantics of the noexcept keyword. See cppreference:

Non-throwing functions are permitted to call potentially-throwing functions. Whenever an exception is thrown and the search for a handler encounters the outermost block of a non-throwing function, the function std::terminate is called

[AlexeySachkov]

The change here seems unnecessary. Even without this change, if an exception is thrown in the body of size, the application will terminate.

My understanding is that one of our code coverage tools complains about throw in noexcept function and we are unable to ignore that by some reason. It is probably a good notification for us to review associated code to see if we can actually make it noexcept, but otherwise I don't think that its the end of the world. All those catch block should almost always be unreachable as I understand it.

[AlexeySachkov]

I've submitted KhronosGroup/SYCL-Docs#626 to see if we want to mark all methods of range/id/other similar classes as noexcept in the SYCL spec so that we can clean up our code here.