[SYCL] Align root_group with spec #12653
Conversation
| return it.get_global_linear_id(); | ||
| } else if constexpr (Scope == execution_scope::sub_group) { | ||
| size_t WIId = it.get_global_linear_id(); | ||
| size_t SGSize = it.get_sub_group().get_local_linear_range(); |
There was a problem hiding this comment.
I think this should be get_max_local_range()[0]. @Pennycook , do I remember correctly what happens when "last" sub-group is partially masked out?
If so, we'd need a test for this as well.
There was a problem hiding this comment.
Agree, will update.
There was a problem hiding this comment.
I'm not 100% sure, but can you do this without a division?
it.get_group().get_group_linear_id() * it.get_sub_group().get_group_linear_range() + it.get_sub_group().get_group_linear_id()There was a problem hiding this comment.
It just occurred to me that this would give a different linearization order, and that the linearization order implied by these functions might not be well-defined yet... Different people might expect them to do different things.
There was a problem hiding this comment.
So, should we proceed with the suggestion above or it requires some clarification to the spec first?
There was a problem hiding this comment.
I think we need some clarification. We should talk to some other folks and figure out what the right thing to do here is.
There was a problem hiding this comment.
@Pennycook in general, do we even want get_linear_id to accept execution_scope::sub_group?
There was a problem hiding this comment.
The issue affects get_id for sub-groups as well.
Because this new API is intended to be a general querying mechanism for all groups, we need to think this through carefully. I'm reluctant to start introducing corner-cases until we understand what's possible. If this API doesn't support sub-groups, it might not support other future group types.
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
|
This pull request is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be automatically closed in 30 days. |
|
This pull request was closed because it has been stalled for 30 days with no activity. |
Spec: #12643