Skip to content

Conversation

@AndreiZibrov
Copy link
Contributor

First part of changes in order of #14604:
fixed_size_group -> chunk

Copy link
Contributor

Choose a reason for hiding this comment

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

I see that we already have this type trait defined in type_traits.hpp, let's to an #include instead of redefining that trait

Copy link
Contributor

Choose a reason for hiding this comment

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

Formatting is broken here

Copy link
Contributor

Choose a reason for hiding this comment

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

sycl_ext_oneapi_non_uniform_groups does not define get_chunk, I suppose you meant chunked_partition

Copy link
Contributor

Choose a reason for hiding this comment

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

We should also leave a TODO comment to implement operator fragment<ParentGroup>() const; once fragment is ready

Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably rename the file as well. Also applies to fixed_size_group_algorithms.cpp

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
template <size_t PartitionSize, typename ParentGroup> class chunk {
template <size_t ChunkSize, typename ParentGroup> class chunk {

That's rather minor, but I think that it is better to be aligned in the terminology we use

@steffenlarsen
Copy link
Contributor

I am concerned about doing the changes in #14604 gradually. I do not mind having multiple patches for them, but they should go in around the same time. We also need to make sure we coordinate these changes with changes to https://github.com/KhronosGroup/SYCL-CTS/tree/SYCL-2020/tests/extension/oneapi_non_uniform_groups.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants