perf(codegen): initialize only the two LLVM backends Perry can emit for - #7350
Merged
Conversation
`Target::initialize_all()` references every LLVM target's init symbol, so
the static link pulls in all ~20 backends. Measured on the `perry`
binary built with the llvm-inprocess feature:
no feature 25.7 MB
feature, initialize_all 185.9 MB (+160.1)
feature, AArch64 + X86 98.9 MB (+73.2)
-86.9 MB, 47% of the whole feature build, for backends nothing can
reach. It was inkwell's convenient default in #7301 rather than a
considered choice, and the feature was opt-in so nobody paid for it --
which matters now that making it standard is on the table.
Perry's LLVM target surface is exactly two architectures: every triple
the compile driver can produce is aarch64 (Apple platforms, Android,
Linux gnu/musl/ohos, and watchOS's ILP32 arm64_32, still the AArch64
backend) or x86 (x86_64, x86_64h, i686). The lone riscv64gc string is a
unit-test assertion in gc_map.rs, not an emission target, and wasm has
its own crate that never reaches this backend.
Fails loudly, never silently: an uninitialized triple errors at
Target::from_triple. A new test asserts all ten driver-producible
triples resolve, so adding an architecture without its backend fails in
cargo-test rather than at a user's compile.
Verified on the 81-module zod corpus: text, transport and native modes
all compile, native output byte-identical to text, RS4GC probe 09
compiles.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughLLVM initialization now registers only AArch64 and x86 targets. A regression test verifies representative Apple, Linux, Android, and Windows triples. ChangesLLVM target initialization
Estimated code review effort: 2 (Simple) | ~10 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/inprocess.rs`:
- Around line 406-434: Update the triple list in
every_supported_triple_resolves_to_an_initialized_backend to include
arm64_32-apple-watchos, ensuring the ILP32 watchOS target is verified as
resolving to an initialized backend when supported. Preserve the existing
global_init and assertion behavior.
🪄 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: ef3e4f35-feac-4e93-8f87-b3eb00a32ad4
📒 Files selected for processing (1)
crates/perry-codegen/src/inprocess.rs
CodeRabbit's catch. The comment claims watchOS's ILP32 arm64_32 is still the AArch64 backend and therefore covered, but the test never asserted it -- the one triple in the list whose coverage is not obvious from its name. Verified: it resolves.
proggeramlug
added a commit
that referenced
this pull request
Aug 4, 2026
…ly linked (#7353) Perry now links LLVM 22 statically and ships self-contained. We own the assumption rather than pushing it onto the user, and there is no "install a compatible clang" step left to get wrong. It is load-bearing, not a preference. The explicit statepoint bridge is gone (#7348), so RS4GC is the only native-root backend, and RS4GC cannot round-trip its IR through an external `opt` plus a different clang (#7339). Keeping this opt-in meant the only working statepoint path was behind a flag nobody sets. Two defaults flip together, because either alone is half a feature: * `llvm-inprocess` becomes a default cargo feature. * `inprocess_requested()` defaults to ON -- but only iff the backend is actually compiled in. Defaulting to `true` unconditionally would route every compile in a `--no-default-features` build into the not-built-in stub and fail it outright. Verified both ways. `PERRY_LLVM_INPROCESS=0` reverts to the clang subprocess for bisection, and `--no-default-features` still builds the text path. CI: a new `.github/actions/setup-llvm22` composite action, referenced from all 44 toolchain steps across 18 workflows. One definition rather than 44 inline recipes, because the three platforms need three different sources and only one is obvious -- Ubuntu 24.04's own llvm-dev is 18, and chocolatey's `llvm` is the clang toolchain with no llvm-config.exe and none of the static libs. Every arm asserts the major version. Size: 98.9 MB, not the 185.9 MB this would have cost before #7350 -- `initialize_all()` was linking ~18 backends nothing can reach. Also fixed, surfaced by the flip: PERRY_LLVM_KEEP_IR promises the whole scratch dir including the .o. The clang path got that free because the object is a file; in-process returns bytes and silently dropped it, degrading a debugging aid exactly when someone is debugging. Verified on the 81-module zod corpus with no env set: compiles, output byte-identical to the clang path, and PERRY_RS4GC=1 now compiles a try-carrying probe with no further flags. 605 codegen tests pass. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
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
1 participant
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.
Target::initialize_all()references every LLVM target's init symbol, so the static link pulls in all ~20 backends.perrybinaryinitialize_all()−86.9 MB — 47% of the whole feature build — for backends nothing can reach.
It was inkwell's convenient default in #7301 rather than a considered choice, and the feature was opt-in so nobody paid for it. That changes now that making
llvm-inprocessstandard is on the table: this is the difference between a 186 MB compiler and a 99 MB one.Why two backends is the honest set. Every triple the compile driver can produce is aarch64 (Apple platforms, Android, Linux gnu/musl/ohos, and watchOS's ILP32
arm64_32— still the AArch64 backend) or x86 (x86_64,x86_64h,i686). The loneriscv64gcstring in the tree is a unit-test assertion ingc_map.rs, not an emission target, and wasm has its own crate (perry-codegen-wasm) that never reaches this backend.Fails loudly, never silently. An uninitialized triple errors at
Target::from_triple("no LLVM target for …"). A new test asserts all ten driver-producible triples resolve, so adding an architecture without initializing its backend fails incargo-testrather than at a user's compile.Verified on the 81-module zod dependency corpus: text, transport and native modes all compile; native output byte-identical to text; RS4GC probe 09 compiles.
Summary by CodeRabbit