Skip to content

emit non-atomic instructions when --single-threaded #1911

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
3 tasks
andrewrk opened this issue Feb 1, 2019 · 5 comments
Open
3 tasks

emit non-atomic instructions when --single-threaded #1911

andrewrk opened this issue Feb 1, 2019 · 5 comments
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Feb 1, 2019

Now that #1764 is implemented, we need to improve language support for it. When building with --single-threaded:

  • @atomicLoad should be a normal (non-atomic) load
  • @atomicRmw should be a normal (non-atomic) read, modify, write
  • @fence should be a no-op
@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. labels Feb 1, 2019
@andrewrk andrewrk added this to the 0.4.0 milestone Feb 1, 2019
@daurnimator
Copy link
Contributor

Just because the current program is single threaded doesn't mean the whole machine is. e.g. you could br using memory mapped or DMAd buffers.
In reply to your IRC comment, I don't think volatile is enough: IIRC volatile in C only ensures that the number of reads/writes in code map exactly to the number of reads/write to memory: it doesn't have any ordering guarantees.

@andrewrk
Copy link
Member Author

andrewrk commented Feb 2, 2019

volatile guarantees order and it is appropriate for DMA. From the zig docs:

Loads and stores are assumed to not have side effects. If a given load or store should have side effects, such as Memory Mapped Input/Output (MMIO), use volatile. In the following code, loads and stores with mmio_ptr are guaranteed to all happen and in the same order as in source code:

It is incorrect to use atomics for DMA or memory mapped I/O.

@andrewrk andrewrk added proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. and removed contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. labels Feb 8, 2019
@andrewrk
Copy link
Member Author

andrewrk commented Feb 8, 2019

I do want to collect more information before committing to this. I recall needing to use @fence even for a single threaded OS kernel:

https://github.com/andrewrk/clashos/blob/e55b6a26a5c01566da14b5f1306822c8da6fc978/src/mmio.zig#L3-L11

@andrewrk andrewrk modified the milestones: 0.4.0, 0.5.0 Feb 8, 2019
@daurnimator
Copy link
Contributor

This also came up in bindings to the linux kernel's uring (#2525).
The kernel has a memory fence on its side, and you have to have a matching one in userspace.

Additionally, .Monotonic atomic operations are merely to ensure ordering of operations so that the compiler doesn't reorder them, this matters for memory mapped IO and lots of low level operations.

Which means:

  • single threaded running under an OS (e.g. linux), still needs to honour atomic operations
  • single threaded that deals with raw device still need to honour atomic operations

I think that's probably every program in the world.... in other words, this issue can be closed.

@andrewrk
Copy link
Member Author

andrewrk commented Aug 2, 2019

One thing to consider is to using the "singlethread" attribute on the atomic operations:

If an atomic operation is marked syncscope("singlethread"), it only synchronizes with and only participates in the seq_cst total orderings of other operations running in the same thread (for example, in signal handlers).

If an atomic operation is marked syncscope("<target-scope>"), where <target-scope> is a target specific synchronization scope, then it is target dependent if it synchronizes with and participates in the seq_cst total orderings of other operations.

Otherwise, an atomic operation that is not marked syncscope("singlethread") or syncscope("<target-scope>") synchronizes with and participates in the seq_cst total orderings of other operations that are not marked syncscope("singlethread") or syncscope("<target-scope>").

@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Aug 28, 2019
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Oct 17, 2019
@andrewrk andrewrk added the accepted This proposal is planned. label Oct 10, 2020
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 10, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 May 19, 2021
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 Nov 20, 2021
@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Apr 16, 2022
@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 Apr 9, 2023
@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Jun 29, 2023
@andrewrk andrewrk modified the milestones: 0.14.0, 0.15.0 Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

2 participants