oneapi-src
[SYCLomatic]Create separate util file for groups
From email discussion on SYCLcompat for cooperative group apis, this is created to add all header algorithms related to block/warp for sycl. This PR only replaces the group apis - exchange and sort (from these PRs : #1562 & #1483 ) which were added to the dpct_extensions.h file to group_utils.h file. No other changes apart from removing the existing code from extensions to the group_utils.h file. Correspondingly the tests related to PRs https://github.com/oneapi-src/SYCLomatic-test/pull/601 & https://github.com/oneapi-src/SYCLomatic-test/pull/594 need to be updated accordingly with respect to the changes in namespace only. The aim of this PR is only to remove the functionality from extensions namespace to group_utils namespace. cc @danhoeflinger @mmichel11 @zhimingwang36 @yihanwg
Is this moving functionality from the other header as is or rewriting + making changes?
If we are moving code and keeping the namespace and everything the same, that is one type of review where we just need to insure that all existing SYCLomatic code can still see the new location, and it is successfully unchanged, etc. If that is the case we need to remove the other code (where it was moved from).
If we are doing more than just moving code, then we need to review it as "new code", and also figure out how it fits with what is already there.
It would be good to make this clear in the description of the PR (without requiring the context of the email discussion to understand it).
Is this moving functionality from the other header as is or rewriting + making changes?
If we are moving code and keeping the namespace and everything the same, that is one type of review where we just need to insure that all existing SYCLomatic code can still see the new location, and it is successfully unchanged, etc. If that is the case we need to remove the other code (where it was moved from).
If we are doing more than just moving code, then we need to review it as "new code", and also figure out how it fits with what is already there.
It would be good to make this clear in the description of the PR (without requiring the context of the email discussion to understand it).
Yes added description - this is mainly a step for removing the existing sort & exchange coop-group apis from dpct extensions namespace to group_utils namespace without changing any functionality. This would require an inclusion of dpcpp_extension.h headers for radix_sort because the detail namespace has radix_rank + twiddle_in operations which are essential for sort. As per @yihanwg review, changes in tests no longer needed as the file is included in dpl_utils.h
please fix the clang format suggestions: https://github.com/oneapi-src/SYCLomatic/actions/runs/8753136412/job/24022223159?pr=1784
Basically LGTM. You should check https://github.com/oneapi-src/SYCLomatic/blob/SYCLomatic/clang/lib/DPCT/HeaderTypes.inc as well to see if it needs to add a new entry.
I don't see any issues within group_utils.hpp header, so once the header addition issues are resolved this PR LGTM.
@yihanwg @zhimingwang36 could you help with the lit failure issue in CI ? Thanks.
@yihanwg @zhimingwang36 could you help with the lit failure issue in CI ? Thanks.
Sure, let me take a look.
[update] Fixed.