fix(codegen): pin apple-m1 on Apple aarch64 instead of -mcpu=native - #7352
Conversation
Two of the three pieces Windows needs are in. The section: COFF is a first-class object format in the emitter now. Its name is .pgcmap, not .perry_gcmap, and that is load-bearing — a PE image section header has an 8-byte name field, and long names survive only in object files as a string-table offset the linker does not carry into the image. The lookup: the runtime finds that section in a running PE image via GetModuleHandleW(NULL) as the image base, walking to the section table past the optional header whose size the file header records. The walker is missing, so Windows stays refused. _Unwind_* does not exist there and the walker module is gated to Apple and Linux, so on Windows it falls to the stub, no frame is visited, and the collector would free live objects. Emitting a map anyway is the silent-lost-roots failure this backend exists to prevent, so the compiler refuses with a message naming the absent piece. A walker means RtlVirtualUnwind or an fp-chain walk, and wants a Windows host to develop against. The PE lookup compiles for x86_64-pc-windows-msvc in isolation; the full crate cannot be cross-checked from macOS because psm/stacker build scripts need a C cross-compiler — the same blocker as watchOS and visionOS.
gc-native-roots has never gone green — 0 successes in 40 runs — and the current cause is not a GC bug: 'Cannot select: intrinsic %llvm.aarch64.fjcvtzs'. inprocess.rs already documents the invariant. Codegen decides whether to emit llvm.aarch64.fjcvtzs (FEAT_JSCVT) from the TRIPLE alone, because clang's default CPU for arm64-apple-* is apple-m1. Anything compiling that IR for a CPU without the feature aborts. The doc names the generic-TargetMachine half of that pair; -mcpu=native is the other half, and it breaks wherever CPU detection disagrees with the triple assumption — which is what a virtualised macOS CI runner does. The same command works on a physical Mac, which is why this only failed in CI. Apple aarch64 hosts now pass an explicit -mcpu=apple-m1, making what we emit and what we target one decision rather than two that agree by luck. Other hosts keep native tuning. Verified on hardware: native_tuning_arg = -mcpu=apple-m1 in the recorded compile plan, and 10/10 probes byte-match the oracle under forced evacuation.
📝 WalkthroughWalkthroughThe change adds COFF/PE stack-map groundwork for Windows, including ChangesWindows GC-map groundwork
Apple arm64 CPU baseline
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GCMapEmitter
participant PEImage
participant StackMapLoader
GCMapEmitter->>PEImage: Emit `.pgcmap` COFF section
StackMapLoader->>PEImage: Parse PE headers and locate `.pgcmap`
PEImage-->>StackMapLoader: Return mapped section address and size
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-codegen/src/gc_map.rs`:
- Around line 1064-1083: Update the test around compact_and_assemble_refusal so
it exercises the production compact_and_assemble path rather than duplicating
its COFF predicate. Assert that compact_and_assemble rejects the Windows target
with the “no stack walker” error, or extract the refusal decision into a shared
helper used by both production code and the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ed46e9e2-4423-4a44-84c5-9955ecf72535
📒 Files selected for processing (6)
changelog.d/7352-windows-groundwork.mdchangelog.d/7353-apple-cpu-baseline.mdcrates/perry-codegen/src/gc_map.rscrates/perry-codegen/src/linker.rscrates/perry-codegen/src/linker_tests.rscrates/perry-runtime/src/gc/roots/stack_maps.rs
| fn compact_and_assemble_refusal(target: &str) -> String { | ||
| // Mirrors the guard in `compact_and_assemble`; kept here so the test | ||
| // fails if that guard is removed rather than if a string changes. | ||
| if matches!(format_for(target), ObjectFormat::Coff) { | ||
| return format!( | ||
| "perry: native GC roots (PERRY_RS4GC) are not enabled for target \ | ||
| `{target}` yet — the runtime has no stack walker on Windows" | ||
| ); | ||
| } | ||
| String::new() | ||
| } | ||
|
|
||
| #[test] | ||
| fn windows_is_refused_until_it_has_a_walker() { | ||
| // The section and its PE lookup exist, but Windows has no stack walker, | ||
| // so every frame would go unvisited and the collector would free live | ||
| // objects. Staged is not enabled. | ||
| let err = compact_and_assemble_refusal("x86_64-pc-windows-msvc"); | ||
| assert!(err.contains("no stack walker"), "{err}"); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Test the production Windows refusal.
compact_and_assemble_refusal duplicates the predicate at Line 922. Production code never calls this helper. If the production COFF refusal is removed, this test still passes.
Extract the target-refusal decision into a shared helper, or call compact_and_assemble from this test and assert its error.
Proposed test structure
- fn compact_and_assemble_refusal(target: &str) -> String {
+ fn native_gc_roots_refusal(target: &str) -> Option<String> {
if matches!(format_for(target), ObjectFormat::Coff) {
- return format!(...);
+ return Some(format!(...));
}
- String::new()
+ None
}
- if matches!(format_for(target), ObjectFormat::Coff) {
- return Err(anyhow!(...));
+ if let Some(reason) = native_gc_roots_refusal(target) {
+ return Err(anyhow!(reason));
}
- let err = compact_and_assemble_refusal("x86_64-pc-windows-msvc");
+ let err = native_gc_roots_refusal("x86_64-pc-windows-msvc")
+ .expect("Windows must remain refused");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn compact_and_assemble_refusal(target: &str) -> String { | |
| // Mirrors the guard in `compact_and_assemble`; kept here so the test | |
| // fails if that guard is removed rather than if a string changes. | |
| if matches!(format_for(target), ObjectFormat::Coff) { | |
| return format!( | |
| "perry: native GC roots (PERRY_RS4GC) are not enabled for target \ | |
| `{target}` yet — the runtime has no stack walker on Windows" | |
| ); | |
| } | |
| String::new() | |
| } | |
| #[test] | |
| fn windows_is_refused_until_it_has_a_walker() { | |
| // The section and its PE lookup exist, but Windows has no stack walker, | |
| // so every frame would go unvisited and the collector would free live | |
| // objects. Staged is not enabled. | |
| let err = compact_and_assemble_refusal("x86_64-pc-windows-msvc"); | |
| assert!(err.contains("no stack walker"), "{err}"); | |
| } | |
| fn native_gc_roots_refusal(target: &str) -> Option<String> { | |
| // Mirrors the guard in `compact_and_assemble`; kept here so the test | |
| // fails if that guard is removed rather than if a string changes. | |
| if matches!(format_for(target), ObjectFormat::Coff) { | |
| return Some(format!( | |
| "perry: native GC roots (PERRY_RS4GC) are not enabled for target \ | |
| `{target}` yet — the runtime has no stack walker on Windows" | |
| )); | |
| } | |
| None | |
| } | |
| #[test] | |
| fn windows_is_refused_until_it_has_a_walker() { | |
| // The section and its PE lookup exist, but Windows has no stack walker, | |
| // so every frame would go unvisited and the collector would free live | |
| // objects. Staged is not enabled. | |
| let err = native_gc_roots_refusal("x86_64-pc-windows-msvc") | |
| .expect("Windows must remain refused"); | |
| assert!(err.contains("no stack walker"), "{err}"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-codegen/src/gc_map.rs` around lines 1064 - 1083, Update the test
around compact_and_assemble_refusal so it exercises the production
compact_and_assemble path rather than duplicating its COFF predicate. Assert
that compact_and_assemble rejects the Windows target with the “no stack walker”
error, or extract the refusal decision into a shared helper used by both
production code and the test.
gc-native-rootshas never gone green — 0 successes in 40 runs. Everything this campaign has shipped was verified by hand on real hardware; the automated gate has never once passed. The current cause is not a GC bug.The invariant this breaks is already written down
inprocess.rsdocuments it:That comment names the generic-TargetMachine half of the pair.
-mcpu=nativeis the other half. It breaks identically wherever CPU detection disagrees with the triple assumption — which is exactly what a virtualised macOS CI runner does. The same command works on a physical Mac, which is why this failed only in CI and looked like a mystery.The fix
Apple aarch64 hosts pass an explicit
-mcpu=apple-m1instead ofnative, matchingdefault_cpu_for_triple. What Perry emits and what it targets become one decision rather than two that happen to agree on developer hardware. Every other host keeps native tuning.Verified on hardware, not inferred
native_tuning_arg = -mcpu=apple-m1in the recorded compile planPERRY_RS4GC=1 PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1This should be what finally lets that gate produce a green run. Once it does, it's worth promoting to required — but only after, never before.
Summary by CodeRabbit
Bug Fixes
Platform Support
Documentation