Repository navigation
fix: disable ThinLTO in test-fast - #1407
Merged
Merged
Conversation
The full macOS workspace gate can fail before execution when large integration test binaries link with explicit ThinLTO under the current rustc, LLVM, and Xcode toolchain. The optimized numerics requirement for the gate depends on opt-level 3, not on cross-crate LTO, and release binaries still use the release profile. This changes only the test-fast LTO setting to lto=false while preserving opt-level 3, codegen-units 16, incremental compilation, unstripped local test binaries, and the inherited release panic behavior. The surrounding Makefile, contributor, installation, and nightly workflow guidance now describe the no-cross-crate-LTO test profile and keep the manual release-codegen escape hatch. Validation: parsed Cargo.toml to confirm test-fast lto=false and release lto=true/codegen-units=1, ran cargo fmt --check, ran git diff --check on the changed files, and linked the molmo_parity and qwen3_omni_moe_parity integration test binaries with cargo test --profile test-fast --features metal,accelerate --test <target> --no-run. Closes #1406
Member
Author
Security/performance review
|
be-free-snowboy
pushed a commit
to freestylejs/snowbrain-mlxcel
that referenced
this pull request
Oct 27, 2026
## Summary - Disable LTO in `[profile.test-fast]` so macOS workspace integration-test binaries no longer enter the ThinLTO link path that produced missing internal Rust symbols. - Preserve the optimized test behavior that matters for MLX numerics: `opt-level = 3`, `codegen-units = 16`, incremental compilation, unstripped local test binaries, and inherited unwind behavior remain unchanged. - Leave `[profile.release]` unchanged with fat LTO and one codegen unit, and update Cargo, Makefile, contributor, installation, and nightly guidance to document the test/release trade-off accurately. - Add bilingual technical reports covering the reproduction, decision, validation evidence, and residual release-codegen gap. ## Root cause The full workspace no-run graph repeatedly failed while linking large integration targets such as `molmo_parity` and `qwen3_omni_moe_parity`, with unresolved LLVM-suffixed Rust drop-glue symbols. The failure reproduced with `CARGO_BUILD_JOBS=1`, while a targeted integration-test build could pass under a smaller unit graph, isolating the failure to the explicit ThinLTO workspace link shape rather than the test source or concurrent Cargo link jobs. ## Validation - TOML profile assertions: `[profile.test-fast]` has `lto = false`, `opt-level = 3`, and `codegen-units = 16`; `[profile.release]` remains `lto = true`, `codegen-units = 1`, `strip = true`, `opt-level = 3`, and `panic = "unwind"`. - `cargo test --profile test-fast --features metal,accelerate --test molmo_parity --no-run` - `cargo test --profile test-fast --features metal,accelerate --test qwen3_omni_moe_parity --no-run` - `cargo test --workspace --profile test-fast --features metal,accelerate --no-run` — passed; all workspace test targets linked in 9m19s on the cold corrected profile. - `cargo test --workspace --profile test-fast --features metal,accelerate --no-fail-fast -- --test-threads=1` — passed with zero failures across all workspace test and doctest binaries. - `cargo clippy --workspace --all-targets --features metal,accelerate -- -D warnings` - `cargo fmt --all -- --check` - `git diff --check origin/main...HEAD` - GitHub CI: formatting, clippy, dependency policy, crate versions, kernel dtype keys, cross-repository references, OpenXLA feature compilation, and CLA checks passed before the report-only commit; the refreshed checks are expected to exercise the same implementation tree. ## Review - Independent correctness review: no findings. - Security/performance review: no findings at any severity; the intentional test-only LTO trade-off and unchanged release workflow were verified. - Finalization review: scope, closing link, profile isolation, documentation, and focused link checks are complete with no blockers. ## Reports - `TECHNICAL_REPORTS/1407-disable-test-thinlto-20260825.en.md` - `TECHNICAL_REPORTS/1407-disable-test-thinlto-20260825.ko.md` Closes lablup#1406
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
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.
Summary
[profile.test-fast]so macOS workspace integration-test binaries no longer enter the ThinLTO link path that produced missing internal Rust symbols.opt-level = 3,codegen-units = 16, incremental compilation, unstripped local test binaries, and inherited unwind behavior remain unchanged.[profile.release]unchanged with fat LTO and one codegen unit, and update Cargo, Makefile, contributor, installation, and nightly guidance to document the test/release trade-off accurately.Root cause
The full workspace no-run graph repeatedly failed while linking large integration targets such as
molmo_parityandqwen3_omni_moe_parity, with unresolved LLVM-suffixed Rust drop-glue symbols. The failure reproduced withCARGO_BUILD_JOBS=1, while a targeted integration-test build could pass under a smaller unit graph, isolating the failure to the explicit ThinLTO workspace link shape rather than the test source or concurrent Cargo link jobs.Validation
[profile.test-fast]haslto = false,opt-level = 3, andcodegen-units = 16;[profile.release]remainslto = true,codegen-units = 1,strip = true,opt-level = 3, andpanic = "unwind".cargo test --profile test-fast --features metal,accelerate --test molmo_parity --no-runcargo test --profile test-fast --features metal,accelerate --test qwen3_omni_moe_parity --no-runcargo test --workspace --profile test-fast --features metal,accelerate --no-run— passed; all workspace test targets linked in 9m19s on the cold corrected profile.cargo test --workspace --profile test-fast --features metal,accelerate --no-fail-fast -- --test-threads=1— passed with zero failures across all workspace test and doctest binaries.cargo clippy --workspace --all-targets --features metal,accelerate -- -D warningscargo fmt --all -- --checkgit diff --check origin/main...HEADReview
Reports
TECHNICAL_REPORTS/1407-disable-test-thinlto-20260825.en.mdTECHNICAL_REPORTS/1407-disable-test-thinlto-20260825.ko.mdCloses #1406