-
Notifications
You must be signed in to change notification settings - Fork 790
[SYCL][ABI Breaking] CG/handler refactoring to reduce boilerplate #9609
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
Conversation
a161d86
to
df464a3
Compare
df464a3
to
18779af
Compare
18779af
to
ee28582
Compare
ee28582
to
6b4c97e
Compare
6b4c97e
to
b59344c
Compare
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.
LGTM overall. Small nit.
sycl/include/sycl/detail/cg.hpp
Outdated
MAccStorage(std::move(AccStorage)), | ||
MSharedPtrStorage(std::move(SharedPtrStorage)), | ||
MRequirements(std::move(Requirements)), MEvents(std::move(Events)) { | ||
struct Data { |
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.
Nit; I'm not sold on the name. "Data" seems too general. Maybe something like CommonCGStorage
?
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.
LGTM for the kernel fusion part, also reviewed the remaining files and did not find any issues.
No description provided.