Skip to content

Specify that atomic_ref member operations with workitem scope are UB.#1021

Open
VerenaBeckham wants to merge 10 commits into
KhronosGroup:mainfrom
VerenaBeckham:verena/workitem_scope
Open

Specify that atomic_ref member operations with workitem scope are UB.#1021
VerenaBeckham wants to merge 10 commits into
KhronosGroup:mainfrom
VerenaBeckham:verena/workitem_scope

Conversation

@VerenaBeckham

Copy link
Copy Markdown
Contributor

Addresses #1004.

Comment thread adoc/chapters/programming_interface.adoc Outdated
@VerenaBeckham VerenaBeckham force-pushed the verena/workitem_scope branch from 0624ba8 to b86ef12 Compare June 24, 2026 10:13
@gmlueck

gmlueck commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

I did a search of the spec for "memory_scope::work_item" to see where else we define behavior for this scope and found:

Both of these have wording like:

Returns: The set of memory scopes supported by atomic operations on this device. When a device returns a "wider" memory scope in this set, it must also return all "narrower" memory scopes. (See Section 3.8.3.2 for a definition of "wider" and "narrower" scopes.) At a minimum, each device must support memory_scope::work_item, memory_scope::sub_group, and memory_scope::work_group.

This has two problems:

  • It says that devices must support memory_scope::work_item for atomic operations, which contradicts the clarification you are adding in this PR, and
  • It says that the query must return all narrower scopes if it returns a wider scope. This won't be true if we simply remove memory_scope::work_item from the list.

@VerenaBeckham

Copy link
Copy Markdown
Contributor Author

I did a search of the spec for "memory_scope::work_item" to see where else we define behavior for this scope and found:

* https://registry.khronos.org/SYCL/specs/sycl-2020/html/sycl-2020.html#api:info-context-atomic-memory-scope-capabilities

* https://registry.khronos.org/SYCL/specs/sycl-2020/html/sycl-2020.html#api:info-device-atomic-memory-scope-capabilities

Both of these have wording like:

Returns: The set of memory scopes supported by atomic operations on this device. When a device returns a "wider" memory scope in this set, it must also return all "narrower" memory scopes. (See Section 3.8.3.2 for a definition of "wider" and "narrower" scopes.) At a minimum, each device must support memory_scope::work_item, memory_scope::sub_group, and memory_scope::work_group.

This has two problems:

* It says that devices must support `memory_scope::work_item` for atomic operations, which contradicts the clarification you are adding in this PR, and

* It says that the query must return all narrower scopes if it returns a wider scope.  This won't be true if we simply remove `memory_scope::work_item` from the list.

You're right. Even if there is no harm in returning memory_scope::work_item from these queries, the way they are described ("scopes supported by atomic operations") implies that they should never return memory_scope::work_item.

I have committed a proposal, which defines a set of acceptable values from which to choose. This, I think, neatly circumvents the issue with the "wider"/"narrower" sentence. Perhaps the way the set is defined could be better worded, but let me know what you think of the approach.

@VerenaBeckham

Copy link
Copy Markdown
Contributor Author

I also added some wording to the definition of the work_item scope. Perhaps not strictly necessary, but could be useful?

Comment thread adoc/chapters/programming_interface.adoc Outdated
Comment thread adoc/chapters/programming_interface.adoc Outdated

@gmlueck gmlueck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! Just a very small comment regarding grammar. Now that the list contains only two things, I think we don't want a comma here.

Comment thread adoc/chapters/programming_interface.adoc Outdated
Comment thread adoc/chapters/programming_interface.adoc Outdated
VerenaBeckham and others added 2 commits June 29, 2026 14:11
Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>
Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>
@TApplencourt

TApplencourt commented Jul 2, 2026

Copy link
Copy Markdown
Contributor
  • Wait for the CTS PR.
  • Ready for Review

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.

3 participants