-
Notifications
You must be signed in to change notification settings - Fork 807
[SYCL] Remove fallback assertions #18310
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
Changes from all commits
7276d95
63d1354
4b60f39
f3448cf
f81a6cc
ba2c467
3273daa
b8d028c
76e7387
4c71a40
b8b616b
6d14b71
fb63485
e1eec5f
e690e43
bab0ad3
47cf1bb
3a0ee98
270c75d
4e76e95
2addd68
8cc1567
ee0707b
1f0df57
bae0683
87119d3
5388760
c1a80b4
38e03a2
b27aedd
18c8076
ebdc467
a619c8f
1684e3f
5aa78ae
2b143af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,13 @@ KernelCompiler/opencl.cpp | |
KernelCompiler/opencl_cache_eviction.cpp | ||
KernelCompiler/opencl_queries.cpp | ||
|
||
# https://github.com/intel/llvm/pull/18310 removed fallback assertions, but due | ||
# to a bug in the L0 drivers, one test causes the implementation to continue | ||
# after assertions are triggered. We allow this regression while the drivers bug | ||
# gets addressed. | ||
# See GSD-11097. | ||
Assert/assert_in_kernels.cpp | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @xtian-github & @gmlueck & @jbrodman - Are you okay with this (hopefully temporary) backwards compatibility issue? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so. It seems like the driver bug is not new. It what scenario is there a loss of functionality compared to the current state of affairs? Is there only a loss of functionality in the follow situation:
In the old implementation, this would work because we use the (slow) fallback approach and bypass the (broken) native support? In the new implementation, the If that's the only compatibility break, I think it's OK. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would be it indeed, with the caveat that the L0 adapter didn't report support for native assertions previous, despite actually supporting them. |
||
|
||
# Likely OK, but need author to provide justification, get approval/confirmation | ||
# from someone: | ||
|
||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -99,11 +99,10 @@ | |
static_assert(__cplusplus >= 201703L, | ||
"DPCPP does not support C++ version earlier than C++17."); | ||
|
||
// Helper macro to identify if fallback assert is needed | ||
#if defined(SYCL_FALLBACK_ASSERT) | ||
#define __SYCL_USE_FALLBACK_ASSERT SYCL_FALLBACK_ASSERT | ||
#else | ||
#define __SYCL_USE_FALLBACK_ASSERT 0 | ||
// MSVC doesn't support #warning and we cannot use other methods to report a | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have a tracker to remove this warning in the future? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure! CMPLRLLVM-70220 |
||
// warning from inside a system header (which SYCL is considered to be). | ||
#if defined(SYCL_FALLBACK_ASSERT) && (!defined(_MSC_VER) || defined(__clang__)) | ||
#warning "SYCL_FALLBACK_ASSERT has been removed and no longer has any effect." | ||
#endif | ||
|
||
#if defined(_WIN32) && !defined(_DLL) && !defined(__SYCL_DEVICE_ONLY__) | ||
|
Uh oh!
There was an error while loading. Please reload this page.