-
Notifications
You must be signed in to change notification settings - Fork 788
[SYCL] Implement SYCL 2020 vec aliases #7889
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
[SYCL] Implement SYCL 2020 vec aliases #7889
Conversation
This commit added aliases for `sycl::vec` class as described in section 4.14.2.2. Aliases of SYCL 2020 specification, revision 6. Aliases which use OpenCL type, such as `cl_int2` are still preserved, because they are used internally, see intel#7888.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Impl LGTM
According to 4.14.2.2. Aliases of SYCL 2020 specification, revision 6, `longlong{N}` aliases do not exists. There is corresponding PR to remove them from the implementation: intel/llvm#7889
/verify with intel/llvm-test-suite#1477 |
According to 4.14.2.2. Aliases of SYCL 2020 specification, revision 6, `longlong{N}` aliases do not exists. There is corresponding PR to remove them from the implementation: intel/llvm#7889
…ement-sycl-2020-vec-aliases
@@ -73,6 +90,9 @@ using ulong = unsigned long; | |||
using longlong = long long; | |||
using ulonglong = unsigned long long; | |||
using half = sycl::detail::half_impl::half; | |||
|
|||
// FIXME: SYCL 2020 spec says that cl_* aliases should reside in sycl::opencl | |||
// namespace. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is blocking us from applying these FIXME comments right away?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently, we still use some of these aliases: #7888. I don't know exactly about cl_*
scalar aliases, but we for sure use cl_*
vector aliases.
I would prefer an incremental approach and get this merged first as a fix for vector aliases and then submit a separate patch for scalar aliases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took another look at this: vector cl_*
aliases depend on scalar cl_*
aliases and those in turn are still used (#7888)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently, we still use some of these aliases: #7888. I don't know exactly about
cl_*
scalar aliases, but we for sure usecl_*
vector aliases.I would prefer an incremental approach and get this merged first as a fix for vector aliases and then submit a separate patch for scalar aliases
I also prefer incremental approach, which should be remove cl_*
alias usages from DPC++ code base first and remove alias definitions after that. This way there will be no technical debt.
This is a follow-up from intel#7889. Adjusted LIT tests to acount for the new definition of long{n} alias, which may not always be equal to sycl::vec<long, n> now.
Post-commit failures are being addressed in #7896 |
I've just realized that some of the failures are SYCL-CTS and with the current spec wording, that is a bug in CTS and not in the implementation. Submitted KhronosGroup/SYCL-CTS#446. We should probably temporarily disable some CTS tests to have green post-commit results |
This is a follow-up from #7889. Adjusted LIT tests to account for the new definition of `long{n}` alias, which may not always be equal to `sycl::vec<long, n>` now. Removed some dead code (type traits) from the runtime.
…suite#1477) According to 4.14.2.2. Aliases of SYCL 2020 specification, revision 6, `longlong{N}` aliases do not exists. There is corresponding PR to remove them from the implementation: intel#7889
This commit added aliases for
sycl::vec
class as described in section 4.14.2.2. Aliases of SYCL 2020 specification, revision 6.Aliases which use OpenCL type, such as
cl_int2
are still preserved, because they are used internally, see #7888.