Correct alignment of atomic types and (re)add Atomic{I,U}128 #55410
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LLVM requires that atomic loads and stores be aligned to at least the size of the type.
rust-highfive
added
the
S-waiting-on-review
RalfJung
reviewed
Oct 30, 2018
| @@ -147,6 +148,9 @@ unsafe impl Sync for AtomicBool {} | |||
| /// This type has the same in-memory representation as a `*mut T`. | |||
| #[cfg(target_has_atomic = "ptr")] | |||
| #[stable(feature = "rust1", since = "1.0.0")] | |||
| #[cfg_attr(target_pointer_width = "16", repr(C, align(2)))] | |||
| #[cfg_attr(target_pointer_width = "32", repr(C, align(4)))] | |||
| #[cfg_attr(target_pointer_width = "64", repr(C, align(8)))] | |||
There was a problem hiding this comment.
Can this not use the ptr_width! defined below?
|
Thanks for taking this. I think it would be worth explicitly documenting what |
|
@ollie27 the documentation already says "this type has the same memory representation as the underlying type X". On one hand, it is already documented, on the other hand one could confuse this statement for saying that it also has the same ABI semantics… hmm. |
|
@bors r+ |
|
📌 Commit 5e50acf has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit 5e50acf with merge 87861469c736fa6dd73508ff7f9da69485239354... |
|
💔 Test failed - status-travis |
bors
added
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Ah, that's my mistake. I never actually tested this on macOS. It looks like |
|
@bors r=pnkfelix |
|
📌 Commit afd8e4c8ad3a05ec13ecb810739bad77c3ba1923 has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit afd8e4c8ad3a05ec13ecb810739bad77c3ba1923 with merge a827be09bf7ffa62c1a7bda3178726197aff7788... |
|
💔 Test failed - status-travis |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
bors
added
S-waiting-on-review
|
@bors r=pnkfelix Missed a location for |
|
📌 Commit 99f7dc4 has been approved by |
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Nov 5, 2018
Correct alignment of atomic types and (re)add Atomic{I,U}128
This is a updated #53514 to also make atomic types `repr(C)` as per comment in #53514 (comment).
Fixes #39590
Closes #53514
r? @pnkfelix
|
☀️ Test successful - status-appveyor, status-travis |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a updated #53514 to also make atomic types
repr(C)as per comment in #53514 (comment).Fixes #39590
Closes #53514
r? @pnkfelix