-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Generic atomic v2 #1477
Generic atomic v2 #1477
Conversation
Here is some discussion in internals about possible designs for extended atomic types: https://internals.rust-lang.org/t/pre-rfc-extended-atomic-types/3068 One thing that is really missing from all |
This RFC has little to do with the design of an |
I don't think this RFC is necessary anymore. See atomic-rs for an example of supporting a generic |
Hear ye, hear ye! This RFC is now entering final comment period. |
In general, it seems useful to be able to query whether atomic operations for a certain size/type are available. |
This was recently added by rust-lang/rust#33048 (should be in the latest nightly). See |
This RFC feels like it is superceded in two respects: one, by rust-lang/rust#33048, and secondly, by #1478, which offers similar capabilities. So I'm currently inclined not to accept. |
For a given type |
@mahkoh Like this: https://github.com/Amanieu/atomic-rs/blob/master/src/nightly.rs#L39 (note that atomic-rs hasn't been updated to use the #[cfg(target_has_atomic)] options yet) |
So you need target specific knowledge and the code has to be updated whenever a target is added. Furthermore, this doesn't handle the case where the architecture allows atomic operations with the alignment not a multiple of the size. |
@mahkoh I've just updated the code, maybe this should make it clearer: https://github.com/Amanieu/atomic-rs/blob/master/src/nightly.rs#L42
No architecture currently supports that and LLVM certainly doesn't since it only supports atomic operations on 8, 16, 32, 64 and 128-bit integer types. |
This seems already outdated because it doesn't handle 128 bit types which you claim LLVM handles. In any case, I don't want to know what the architecture supports or what LLVM supports. I don't want to do my own research. The compiler has already done it for me. So I want the compiler to tell me. |
128-bit atomic support is blocked on RFC #1504. And I don't think any architecture will ever support unaligned atomics since that involves atomic operations crossing cache line boundaries. |
How is it blocked on that RFC? Just because there are no 128 bit integers doesn't mean that there aren't any 128 bit aligned 128 types. |
LLVM atomic instructions only accept integer types, and the Rust atomic intrinsics were changed to also only accept integer types. This means that you need a 128-bit integer to perform a 128-bit atomic operation. I guess you could add a hack in the atomic intrinsics to support 128-bit atomics with a non-integer type, but I feel it's not worth the effort (not much demand for 128-bit atomics) so we might as well wait for full 128-bit integer support. |
I see that the situation is worse than I initially assumed. The existence of atomic intrinsics for arbitrary types is of course necessary for this RFC to make any sense. So the RFC has to be amended with the requirement that the intrinsics translate arbitrary types to fitting integer types themselves. |
We discussed in the @rust-lang/lang meeting and we have decided not to accept this RFC. In particular, the @rust-lang/libs team also recently visited the question of atomics in the form of #1543, and decided to adopt the approach advocated there, which seems to somewhat obviate the need for this approach. Thanks @mahkoh! |
Add compiler support for generic atomic operations.