Skip to content

Clarify atomics memory model - #1032

Open
VerenaBeckham wants to merge 5 commits into
KhronosGroup:mainfrom
VerenaBeckham:verena/atomics_clarification
Open

Clarify atomics memory model#1032
VerenaBeckham wants to merge 5 commits into
KhronosGroup:mainfrom
VerenaBeckham:verena/atomics_clarification

Conversation

@VerenaBeckham

Copy link
Copy Markdown
Contributor

Intended to replace #849. Based on text from OpenCL: https://gitlab.khronos.org/opencl/OpenCL-Docs/-/merge_requests/226.

Comment thread adoc/chapters/architecture.adoc
Comment thread adoc/chapters/architecture.adoc Outdated
2.0 specification>>.
For example: sequential consistency must be defined in a way that accounts for
atomic operations with differing (but compatible) scopes, in a manner similar to
the <<opencl20, OpenCL 2.0 specification>>.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't know if there is a better way to refer to the OpenCL spec, which I believe is also a combined spec? Or should we just add a reference to the 3.1 spec?

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.

I think a link to 3.3.7. Memory Consistency Model for OpenCL 2.x maybe the cleaner indeed .

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To be honest, I'm not sure what this sentence is trying to convey other than a TODO for the SYCL WG. Similarly, the following sentence is a promise of something we are still not doing. Is anyone opposed to just removing both lines?

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.

I agree that this whole NOTE is a TODO for the WG. What do we need to do to address it? Since this PR is already addressing some memory model things, can it be expanded to address the remaining issues in this NOTE?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As per WG discussion I have removed the rest of the note. I opened #1041 to track addressing the TODOs described in it.

Comment thread adoc/chapters/architecture.adoc Outdated
Comment thread adoc/chapters/architecture.adoc Outdated
Comment thread adoc/chapters/architecture.adoc Outdated
Comment on lines 1126 to 1130
[NOTE]
====
The addition of memory scopes to the {cpp} memory model modifies the definition
of some concepts from the {cpp} core language.
For example: data races, the synchronizes-with relationship and sequential
consistency must be defined in a way that accounts for atomic operations with
differing (but compatible) scopes, in a manner similar to the <<opencl20, OpenCL
2.0 specification>>.
Efforts to formalize the memory model of SYCL are ongoing, and a formal memory
model will be included in a future version of the SYCL specification.
====

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.

Move the note to issue.
Thanks!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done: #1041


In SYCL 2020 the memory scope of _A_ and _B_ must additionally be the same for
the operations to have an inclusive scope.
From SYCL {SYCL_VERSION} the memory scope of _A_ and _B_ may be different.

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.

I wonder if we should add a non-normative note here to explain the rationale. Something like:

[Note: The SYCL definition of inclusive scope follows the OpenCL definition. In OpenCL versions prior to 3.1, the memory scopes of A and B must be the same for their scopes to be inclusive, but this was relaxed in OpenCL version 3.1. As a result, implementations of SYCL 2020 using an OpenCL backend may use the older OpenCL semantic, but implementations of {SYCL_VERSION} must use the OpenCL 3.1 semantic. -- end note]

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