[SYCL] Disable printing in SYCL headers when compiling for device#22236
[SYCL] Disable printing in SYCL headers when compiling for device#22236vmustya wants to merge 2 commits into
Conversation
The SYCL device/JIT environment don't support printing, so disable it in the SYCL headers when compiling for device. This is needed to avoid compilation errors when the headers are included in device code.
sarnex
left a comment
sarnex
left a comment
There was a problem hiding this comment.
adding others to review this on content, not qualified myself
| inline namespace _V1 { | ||
|
|
||
| inline std::ostream &operator<<(std::ostream &Out, backend be) { | ||
| #if !defined(__SYCL_DEVICE_ONLY__) |
There was a problem hiding this comment.
Can we add a test, either e2e or lit, to lock this down? Otherwise someone could break it and we'd have no idea for a while
There was a problem hiding this comment.
I have really no ideas how to add tests for these particular changes.
There was a problem hiding this comment.
Maybe dump the device IR and look for string consts? Maybe others have a better idea
There was a problem hiding this comment.
A lot of what you are doing in this I implemented on #22047 and move those functions at the src runtime.
|
|
||
| // to output exceptions caught in ~destructors | ||
| #ifndef NDEBUG | ||
| #if !defined(NDEBUG) && !defined(__SYCL_DEVICE_ONLY__) |
There was a problem hiding this comment.
This looks like a fixed function signature. Can we have a runtime function that has some signature like:
report_exception_error(std::string_view, const char*);
If device compilation ever reaches here (which it shouldn't you should get a link error, and you can verify this, in the tests).
| @@ -67,6 +68,7 @@ inline void defaultAsyncHandler(exception_list Exceptions) { | |||
| } | |||
| } | |||
| std::cerr << std::endl; | |||
| #endif // !defined(__SYCL_DEVICE_ONLY__) | |||
There was a problem hiding this comment.
Move the entire function into the runtime, leave declaration and add __SYCL_EXPORT.
| std::cout << "parallel_for range adjusted at dim " << Dim << " from " | ||
| << RoundedRange[Dim] << " to " << Value << std::endl; |
There was a problem hiding this comment.
Extend: sycl/source/detail/range_rounding.cpp with a reportAdustedRange() function that prints out the same message.
koparasy
left a comment
koparasy
left a comment
There was a problem hiding this comment.
To avoid macros overall I think these functions should move in the runtime in the first place. Longer term these headers will not need to pull in iostream (even when doing host compilation). I have some suggestions on how to get there.
I believe if you do it this way you can have "negative" tests, have device only codes that directly call these functions, which should result into linkage errors. As there are no definitions of the suggested functions.
3 participants
The SYCL device/JIT environment don't support printing, so disable it in
the SYCL headers when compiling for device. This is needed to avoid
compilation errors when the headers are included in device code.