-
Couldn't load subscription status.
- Fork 108
cpp 17: Simplify launch implementations if constexpr #1912
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
|
Is there anything to do for the OpenMP backend? |
I couldn't identify an easy simplification for the OpenMP backend. We could wrap different pragmas with if constexpr, but I don't think it would save that much duplication |
9bab1a8 to
8cbc7c4
Compare
6ef56d4 to
21a74dd
Compare
| struct is_ForallParamPack_empty<ForallParamPack<>> : std::true_type | ||
| {}; | ||
| } // namespace type_traits | ||
|
|
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 we should move this to a "params utils" file?
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 did something similar--I moved all our type trait stuff into its own header
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.
Look great! Just 1 comment on where to place helpers.
|
@artv3 I thought a bit more about our type traits and reorganized the headers a bit. I remembered I made a type traits header for kernel-specific work. I renamed that to be more consistent, and made another TypeTraits.hpp for reduction work. Let me know what you think |
This PR simplifies some of the launch SFINAE for cuda/hip. It also moves constexpr branching into parampack calls, simplifying the forall/launch implementations somewhat.