Skip to content

Refactor scalar reductions to use common execution policy#573

Merged
jjwilke merged 6 commits intonv-legate:branch-22.10from
jjwilke:common-exec-policy-cherry-pick
Sep 19, 2022
Merged

Refactor scalar reductions to use common execution policy#573
jjwilke merged 6 commits intonv-legate:branch-22.10from
jjwilke:common-exec-policy-cherry-pick

Conversation

@jjwilke
Copy link
Contributor

@jjwilke jjwilke commented Sep 2, 2022

Following the design doc for improving code reuse, this implements all scalar reduction kernels through a single implementation.

  • Study verifies binary size is not increased
  • Study verifies performance is equivalent

@jjwilke jjwilke added the category:improvement PR introduces an improvement and will be classified as such in release notes label Sep 2, 2022
@magnatelee
Copy link
Contributor

@jjwilke please add a copyright header to the new files

@magnatelee
Copy link
Contributor

left two nitpick-y comments. otherwise looks good.

@jjwilke
Copy link
Contributor Author

jjwilke commented Sep 6, 2022

@magnatelee Should be ready to merge. There was an additional change required to unary_red_util.h you might want to glance at.

@jjwilke jjwilke requested a review from manopapad September 6, 2022 16:17
#define CUDA_FUNCTION
#endif

#endif // SRC_CUNUMERIC_EXECUTION_POLICY_EXECUTION_POLICY_HELPERS_H_ No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing newline at end of file. Weird that the clang auto-formatter didn't catch that. Does it look like clang-format is running when you make change to header files? It might be that the filename pattern is skipping header files, or maybe just our clang-format configuration isn't checking for missing newlines at end of files.

@magnatelee
Copy link
Contributor

@magnatelee Should be ready to merge. There was an additional change required to unary_red_util.h you might want to glance at.

Feel free to merge this once you remove the obsolete header file.

@jjwilke jjwilke merged commit e5f90b7 into nv-legate:branch-22.10 Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:improvement PR introduces an improvement and will be classified as such in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants