-
Notifications
You must be signed in to change notification settings - Fork 104
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
Remove deprecated SYCL usage for newer compilers #1772
Conversation
This is good to merge. Please review when you have some time. Thank you. |
include/RAJA/policy/sycl/policy.hpp
Outdated
@@ -39,7 +39,7 @@ struct uint3 { | |||
unsigned long x, y, z; | |||
}; | |||
|
|||
using sycl_dim_t = cl::sycl::range<1>; | |||
using sycl_dim_t = sycl::range<1>; |
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.
Maybe keep the global scope ::sycl
for consistency?
using sycl_dim_t = sycl::range<1>; | |
using sycl_dim_t = ::sycl::range<1>; |
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.
Good catch. Fixed.
Note that I didn't exhaustively search the code for these. I only changed the lines that generated compiler warnings.
device = reinterpret_cast<T *>(::sycl::malloc_device(sycl::MaxNumTeams * sizeof(T), *(q))); | ||
host = reinterpret_cast<T *>(::sycl::malloc_host(sycl::MaxNumTeams * sizeof(T), *(q))); |
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.
The references to MaxNumTeams
look slightly confusing; should they be under the RAJA::sycl::
namespace?
device = reinterpret_cast<T *>(::sycl::malloc_device(sycl::MaxNumTeams * sizeof(T), *(q))); | |
host = reinterpret_cast<T *>(::sycl::malloc_host(sycl::MaxNumTeams * sizeof(T), *(q))); | |
device = reinterpret_cast<T *>(::sycl::malloc_device(RAJA::sycl::MaxNumTeams * sizeof(T), *(q))); | |
host = reinterpret_cast<T *>(::sycl::malloc_host(RAJA::sycl::MaxNumTeams * sizeof(T), *(q))); |
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.
Note that I did not write this code and was fixing compiler warnings. It looks like this file was started as a copy of the similarly named openmp target file.
This is probably a better was to define the variable: https://github.com/LLNL/RAJA/blob/develop/include/RAJA/policy/openmp_target/reduce.hpp#L70
However, that file also contains similar namespace scoping issues. For example, https://github.com/LLNL/RAJA/blob/develop/include/RAJA/policy/openmp_target/reduce.hpp#L107
I'll make an issue and we can fix those files and others for consistency as needed in a separate PR.
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'm having the same build failure (OMP Target) on a different RAJA PR.... |
Try restarting the failed job. One test fails intermittently. Not sure why. |
Summary