Tracking issue for pref_align_of intrinsic #91971
Comments
RalfJung
added
the
C-tracking-issue
Mark-Simulacrum
added
T-lang
joshtriplett
added
S-tracking-design-concerns
|
Context for labels applied: it sounds like we need a summary of what precisely this means, as distinct from |
|
cc @daltenty @gilamn5tr @mustartt I noticed that the latest change to the notion of "preferred alignment" in clang was for AIX: llvm/llvm-project@05ad8e9 I found the differential revision's commentary difficult to follow: https://reviews.llvm.org/D79719 But I do have a strong sense now that "preferred alignment" is a misnomer, here. I have been doing a lot of work on the layout handling in our backend lately and I feel I should probably have at least a partial understanding of this, as it may impact the layout algorithm used for |
|
Thanks for taking a look into this. We have actually also been taking an interest in the layout algorithm of (Borrowing from https://reviews.llvm.org/D54814's explanation) clang and gcc provide queries which may return two different alignment values for a type:
There's no hard and fast definition of "preferred alignment" but conventionally it is the ideal we'd like to provide for the type if we have the freedom to do so when allocating it (which may be different from the minimum alignment, guaranteed by the platform ABI, that any access can assume to be true). Our understanding is that "preferred alignment" would be non-ABI affecting for C, however for C++ there is one known exception (referred to in https://reviews.llvm.org/D79719) where the preferred alignment is used in the ABI for 32-bit C++ on AIX for calculating the operator new cookie placement. (Not sure how relevant that is for Rust, but here for completeness) Coming back to the "Power Alignment" rules, these rules are the default layout and alignment rules used by the C ABI on AIX (and historically some other Power platforms). A general description of this is given in the IBM XL compiler manual (https://www.ibm.com/docs/en/xl-c-and-cpp-aix/16.1?topic=data-using-alignment-modes), though it is light on details when it comes to nested aggregates and other interesting cases. In general, there are special cases for first member doubles and similar types which need to be observed (recursively in some cases) to obtain a C ABI compatible layout. |
Oh, that should not be relevant to us at all, huzzah. |
|
@daltenty Currently our layout algorithms blithely assume that layout is architecture-specific, but while architecture is the strongest voice, it isn't the only one. To conform to the ABI described would require an AIX-specific layout algorithm. |
|
Yeah, that is definitely a requirement for true C interoperability. Both the architecture and the platform's layout rules must be accounted for. The current design of There's a fairly lengthy thread here debating about how to handle that: https://internals.rust-lang.org/t/repr-c-aix-struct-alignment/21594/36 |
|
After much discussion we have discovered that AIX's "weird" layout rules are actually a mistake in terms of the spec for the target and not a concern in the slightest. |
In rust-lang#90877 (comment) T-lang decided they did not wish to remove `intrinsics::pref_align_of`. However, the intrinsic and its supporting code 1. is a nightly feature, so can be removed at compiler/libs discretion 2. requires considerable effort in the compiler to support, as it necessarily complicates every single site reasoning about alignment 3. has been justified based on relevance to codegen, but it is only a requirement for C++ (not C, not Rust) stack frame layout for AIX[^1], in ways Rust would not consider even with increased C++ interop 4. is only used by rustc to overalign some globals[^3], not correctness 5. can be adequately replaced by other rules for globals, as it mostly affects alignments for a few types under 16 bytes of alignment 6. has only one clear benefactor: automating C -> Rust translation for GNU extensions[^4] like `__alignof` 7. was likely intended to be `alignof`, `_Alignof`, AKA `mem::align_of`, because the GNU extension is a "false friend" of the C keyword, which makes the choice to support such a mapping very questionable 8. makes it easy to do incorrect codegen with the compiler by its mere presence, as usual Rust rules of alignment (e.g. size == align * N) do not hold with preferred alignment[^5] The implementation is clearly damaging the code quality of the compiler. Thus it is within the compiler team's purview to simply rip it out. If T-lang wishes to have this intrinsic restored for c2rust's benefit, it would have to use a radically different implementation that somehow does not cause internal incorrectness. Good luck in implementing that. Until then, remove the intrinsic and its supporting code, as one tool and an ill-considered GCC extension cannot justify risking correctness. Because we touch a fair amount of the compiler to change this at all, and unfortunately the duplication of AbiAndPrefAlign is deep-rooted, we keep an "AbiAlign" type which we can wean code off later. [^1]: rust-lang#91971 (comment) [^2]: rust-lang#135552 [^3]: as viewable in the code altered by this PR [^4]: c2rust: https://github.com/immunant/c2rust/blame/3b1ec86b9b0cf363adfd3178cc45a891a970eef2/c2rust-transpile/src/translator/mod.rs#L3175 [^5]: rust-lang/rustc_codegen_cranelift#1560
…r=bjorn3 Remove rustc's notion of "preferred" alignment AKA `__alignof` In PR rust-lang#90877 T-lang decided not to remove `intrinsics::pref_align_of`. However, the intrinsic and its supporting code 1. is a nightly feature, so can be removed at compiler/libs discretion 2. requires considerable effort in the compiler to support, as it necessarily complicates every single site reasoning about alignment 3. has been justified based on relevance to codegen, but it is only a requirement for C++ (not C, not Rust) stack frame layout for AIX[^1], in ways Rust would not consider even with increased C++ interop 4. is only used by rustc to overalign some globals, not correctness[^2] 5. can be adequately replaced by other rules for globals, as it mostly affects alignments for a few types under 16 bytes of alignment 6. has only one clear beneficiary: automating C -> Rust translation for GNU extensions like `__alignof`[^3] 7. such code was likely intended to be `alignof` or `_Alignof`, because the GNU extension is a "false friend" of the C keyword, which makes the choice to support such a mapping very questionable 8. makes it easy to do incorrect codegen in the compiler by its mere presence as usual Rust rules of alignment (e.g. `size == align * N`) do not hold with preferred alignment[^4] Despite an automated translation tool like c2rust using it, we have made multiple attempts to find a crate that actually has been committed to public repositories, like GitHub or crates.io, using such translated code. We have found none. While it is possible someone privately uses this intrinsic, it seems unlikely, and it is behind a feature gate that will warn about using the internal features of rustc. The implementation is clearly damaging the code quality of the compiler. Thus it is within the compiler team's purview to simply rip it out. If T-lang wishes to have this intrinsic restored for c2rust's benefit, it would have to use a radically different implementation that somehow does not cause internal incorrectness. Until then, remove the intrinsic and its supporting code, as one tool and an ill-considered GCC extension cannot justify risking correctness. Because we touch a fair amount of the compiler to change this at all, and unfortunately the duplication of AbiAndPrefAlign is deep-rooted, we keep an "AbiAlign" type which we can wean code off later. [^1]: rust-lang#91971 (comment) [^2]: as viewable in the code altered by this PR [^3]: c2rust: https://github.com/immunant/c2rust/blame/3b1ec86b9b0cf363adfd3178cc45a891a970eef2/c2rust-transpile/src/translator/mod.rs#L3175 [^4]: rust-lang/rustc_codegen_cranelift#1560
…r=bjorn3 Remove rustc's notion of "preferred" alignment AKA `__alignof` In PR rust-lang#90877 T-lang decided not to remove `intrinsics::pref_align_of`. However, the intrinsic and its supporting code 1. is a nightly feature, so can be removed at compiler/libs discretion 2. requires considerable effort in the compiler to support, as it necessarily complicates every single site reasoning about alignment 3. has been justified based on relevance to codegen, but it is only a requirement for C++ (not C, not Rust) stack frame layout for AIX[^1], in ways Rust would not consider even with increased C++ interop 4. is only used by rustc to overalign some globals, not correctness[^2] 5. can be adequately replaced by other rules for globals, as it mostly affects alignments for a few types under 16 bytes of alignment 6. has only one clear beneficiary: automating C -> Rust translation for GNU extensions like `__alignof`[^3] 7. such code was likely intended to be `alignof` or `_Alignof`, because the GNU extension is a "false friend" of the C keyword, which makes the choice to support such a mapping very questionable 8. makes it easy to do incorrect codegen in the compiler by its mere presence as usual Rust rules of alignment (e.g. `size == align * N`) do not hold with preferred alignment[^4] Despite an automated translation tool like c2rust using it, we have made multiple attempts to find a crate that actually has been committed to public repositories, like GitHub or crates.io, using such translated code. We have found none. While it is possible someone privately uses this intrinsic, it seems unlikely, and it is behind a feature gate that will warn about using the internal features of rustc. The implementation is clearly damaging the code quality of the compiler. Thus it is within the compiler team's purview to simply rip it out. If T-lang wishes to have this intrinsic restored for c2rust's benefit, it would have to use a radically different implementation that somehow does not cause internal incorrectness. Until then, remove the intrinsic and its supporting code, as one tool and an ill-considered GCC extension cannot justify risking correctness. Because we touch a fair amount of the compiler to change this at all, and unfortunately the duplication of AbiAndPrefAlign is deep-rooted, we keep an "AbiAlign" type which we can wean code off later. [^1]: rust-lang#91971 (comment) [^2]: as viewable in the code altered by this PR [^3]: c2rust: https://github.com/immunant/c2rust/blame/3b1ec86b9b0cf363adfd3178cc45a891a970eef2/c2rust-transpile/src/translator/mod.rs#L3175 [^4]: rust-lang/rustc_codegen_cranelift#1560
Rollup merge of #141803 - workingjubilee:remove-pref-align, r=bjorn3 Remove rustc's notion of "preferred" alignment AKA `__alignof` In PR #90877 T-lang decided not to remove `intrinsics::pref_align_of`. However, the intrinsic and its supporting code 1. is a nightly feature, so can be removed at compiler/libs discretion 2. requires considerable effort in the compiler to support, as it necessarily complicates every single site reasoning about alignment 3. has been justified based on relevance to codegen, but it is only a requirement for C++ (not C, not Rust) stack frame layout for AIX[^1], in ways Rust would not consider even with increased C++ interop 4. is only used by rustc to overalign some globals, not correctness[^2] 5. can be adequately replaced by other rules for globals, as it mostly affects alignments for a few types under 16 bytes of alignment 6. has only one clear beneficiary: automating C -> Rust translation for GNU extensions like `__alignof`[^3] 7. such code was likely intended to be `alignof` or `_Alignof`, because the GNU extension is a "false friend" of the C keyword, which makes the choice to support such a mapping very questionable 8. makes it easy to do incorrect codegen in the compiler by its mere presence as usual Rust rules of alignment (e.g. `size == align * N`) do not hold with preferred alignment[^4] Despite an automated translation tool like c2rust using it, we have made multiple attempts to find a crate that actually has been committed to public repositories, like GitHub or crates.io, using such translated code. We have found none. While it is possible someone privately uses this intrinsic, it seems unlikely, and it is behind a feature gate that will warn about using the internal features of rustc. The implementation is clearly damaging the code quality of the compiler. Thus it is within the compiler team's purview to simply rip it out. If T-lang wishes to have this intrinsic restored for c2rust's benefit, it would have to use a radically different implementation that somehow does not cause internal incorrectness. Until then, remove the intrinsic and its supporting code, as one tool and an ill-considered GCC extension cannot justify risking correctness. Because we touch a fair amount of the compiler to change this at all, and unfortunately the duplication of AbiAndPrefAlign is deep-rooted, we keep an "AbiAlign" type which we can wean code off later. [^1]: #91971 (comment) [^2]: as viewable in the code altered by this PR [^3]: c2rust: https://github.com/immunant/c2rust/blame/3b1ec86b9b0cf363adfd3178cc45a891a970eef2/c2rust-transpile/src/translator/mod.rs#L3175 [^4]: rust-lang/rustc_codegen_cranelift#1560
|
#141803 removed this intrinsic, so the issue can be closed. |
Uh oh!
There was an error while loading. Please reload this page.
This intrinsic returns the "preferred" alignment of a type, which can be different from the minimal alignment exposed via
mem::align_of. It is not currently exposed through any wrapper method, but can still be accessed by unstable code using theintrinsicsorcore_intrinsicsfeatures.See #90877 for why it cannot be removed; the short summary is some code is actually using it. So let's have a place to centralize discussion about the intrinsics. (This is not a regular tracking issue because there is no associated feature gate, but close enough I think.)
It would be good to figure out what the "preferred alignment" actually is used for, e.g. there doesn't seem to be consensus on the question whether it is ever relevant for ABI or UB, or whether it is merely a hint used by the compiler to align some globals more than it has to.
The text was updated successfully, but these errors were encountered: