powerpc-apple-darwin: fix the ABI - #421
Open
barracuda156 wants to merge 4 commits into
Open
Conversation
`ents[0].align` got tag_size and `ents[0].size` got tag_align. The two are identical for every power-of-two integer tag on most targets, so this stayed invisible, but it is wrong wherever a tag's size and alignment differ - e.g. a repr(u64) enum with data on i686 (size 8, align 4). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0179wmF1vKBsFua4Wh36WFc5 Signed-off-by: Sergey Fedorov <vital.had@gmail.com>
Replaces the attribute-pinning approach: instead of laying types out by
mrustc's own rules and forcing the C compiler to match via
__attribute__((aligned))/packed, replicate GCC's layout algorithm
(ADJUST_FIELD_ALIGN + darwin_rs6000_special_round_type_align) so the
plain emitted C is already identical - and so repr(C) types keep
matching system headers, which GCC compiles *without* those attributes.
Differences from the pinned model:
- The cap is gated on Darwin (Target_IsDarwinPPC32: arch AND os). Other
32-bit PowerPC OSes don't use the power alignment rule (SysV keeps
long long/double 8-aligned in structs, matching rustc's i64:64
datalayout for powerpc-unknown-linux-gnu), and ppc64 Darwin is
natural-aligned.
- Unions follow GCC: members are capped like struct fields, then the
union's alignment is raised to its *first* member's natural alignment
(darwin_rs6000_special_round_type_align covers unions too). Pinning
every union with an explicit aligned attribute diverges from the
platform ABI: GCC gives union{uint32_t;uint64_t} align 4, the pinned
version align 8, so any repr(C) struct embedding such a union no
longer interoperates with C.
- Every field is capped (including the first); the record's total
alignment is then raised to the natural alignment of its "innermost
first field" (descend the first *emitted* member of each aggregate,
stripping arrays). Equivalent to a first-field exemption for plain
structs, and also correct when the first member is a union or its
alignment comes from a kept 16-byte item.
- TYPE_USER_ALIGN is modelled against the emitted C
(darwin_ppc32_type_has_c_user_align mirrors codegen's
has_manual_align conditions, including alignment carried by ZST
fields, which are not emitted as C fields and therefore cannot
propagate user-alignment in GCC's eyes).
- Enum reprs get a final fixup computing size/align the way the C
compiler sees the emitted struct e_X - variant structs are capped
when they become DATA union members. This subsumes the
final-variant-layout fix for niche enums and covers tagged enums too.
- repr(align(N)) structs always emit an explicit aligned attribute: C
cannot infer a user alignment that happens to equal the largest field
alignment, and on Darwin ppc32 the derived alignment can be lower
than repr->align.
Verified against GCC 16 (iains/gcc-16-branch, the toolchain actually
used on PowerPC Macs) with a hand-checked layout matrix; the sizeof
static asserts stay enabled, and a full libstd builds and runs on
10.6/ppc with debug assertions on - no MINICARGO_NO_DEBUG_ASSERTIONS
needed.
Keeps the empty target_env and the libatomic linkage from the previous
implementation.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0179wmF1vKBsFua4Wh36WFc5
Signed-off-by: Sergey Fedorov <vital.had@gmail.com>
std's atomics module documents ("Portability"): "All atomic types in
this module are guaranteed to be lock-free if they're available. This
means they don't internally acquire a global mutex." 32-bit PowerPC has
no 8-byte atomic instructions, so libatomic implements them with a
lock - exposing AtomicU64 on top of that breaks the documented
contract, and lock-based atomics are not address-free (unusable across
shared memory and in signal handlers).
rustc agrees: every 32-bit powerpc target sets max_atomic_width 32, and
rustc ships libstd for powerpc-unknown-linux-gnu built exactly that
way - the AtomicU64 uses in std are cfg(target_has_atomic)-gated.
The one exception in 1.74 libstd is the mach_timebase_info cache
(sys/unix/time.rs), which packs numer/denom into a single AtomicU64
with no fallback - upstream rustc could assume 64-bit atomics on every
Apple target it supported. Patch it to a pair of AtomicU32s: denom == 0
is still the uninitialized sentinel, and the numer store is published
by the Release store of denom (racing initializers write identical
values). With that, libstd 1.74 builds without AtomicU64.
Drops the now-unneeded -latomic from the powerpc-apple-darwin spec.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0179wmF1vKBsFua4Wh36WFc5
Signed-off-by: Sergey Fedorov <vital.had@gmail.com>
libdispatch exists on Mac OS X 10.6, and 10.6 runs on PowerPC - "powerpc-apple-darwin is by definition 10.4/10.5" doesn't hold, and a 10.6/ppc build wants the dispatch-based Darwin parker. Availability is a property of the deployment target, not the architecture, and std has no cfg for the OS version - so make the fallback opt-in: the std patch keys on `--cfg no_libdispatch`, and the stable-1.74.0-macos-powerpc build_std override sets it (that override targets 10.4/10.5). A 10.6+ build uses an override without the cfg and keeps the dispatch parker. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0179wmF1vKBsFua4Wh36WFc5 Signed-off-by: Sergey Fedorov <vital.had@gmail.com>
Comment on lines
-993
to
-1006
|
|
||
| bool make_field_ent(const Span& sp, const StaticTraitResolve& resolve, unsigned idx, ::HIR::TypeRef ty, Ent& out) | ||
| { | ||
| size_t size, align; | ||
| if( !Target_GetSizeAndAlignOf(sp, resolve, ty, size, align) ) | ||
| { | ||
| DEBUG("Can't get size/align of " << ty); | ||
| return false; | ||
| } | ||
| out = Ent { idx, size, align, HIR::TypeRef(), false }; | ||
| out.user_align = Target_TypeHasUserAlignment(sp, resolve, ty); | ||
| out.ty = mv$(ty); | ||
| return true; | ||
| } |
Owner
There was a problem hiding this comment.
Why was this helper (and its use) removed?
Even if user_align is now gone, the rest seemed pretty useful
Contributor
Author
There was a problem hiding this comment.
It was added in powerpc-darwin-specific commit (which does not fully match the ABI), and not used in updated code (or outside of ppc). I do not insist on removing anything, of course; if we make a rewrite that still works correctly, it will be perfectly fine.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
A follow-up to #418 that actually matches the ABI correctly (no need to disable asserts anymore) and fixes a few minor bugs.
The
mrustcbuild is verified on the real hardware and confirmed to work.rustcbootstrap is still broken, as it was, nothing changes here, but it is broken on unrelated issues (basically on LLVM).P. S. We still need to address unwinding issue to fix the build on 10.5 (which has no
libunwindin the system) and fix libs for ppc64 (broken currently).On 10.6 the working native
mrustccan be built right now from https://github.com/macos-powerpc/powerpc-ports/blob/c3731ad2d6bb8569e35f8446c134bcadeea02394/lang/mrustc/Portfile and verified via building several Rust ports, see macos-powerpc/powerpc-ports#62 for details.@danifunker FYI