ci: run lance-linalg's release tests on the runner's own CPU - #8844
Open
LuciferYang wants to merge 1 commit into
Open
ci: run lance-linalg's release tests on the runner's own CPU#8844LuciferYang wants to merge 1 commit into
LuciferYang wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
The change correctly reuses the x86-64-v2 release artifact, preserves the Nehalem fallback/SIGILL run, and adds the native execution without recompiling.
The remaining risk is intentional: GitHub does not contract an AVX-capable runner, so a green native run does not by itself prove that a SIMD arm executed. The existing feature report makes the actual tier visible in the log; no further change is requested for this pull request.
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.
What this changes
qemu-pre-haswellalready buildslance-linalg's lib tests in release mode. This adds one step that runs that same binary a second time, on the runner's own CPU instead of under qemu, by overriding the cargo runner toenvfor that step only. The build step and the new run move above the QEMU setup steps, and the job'sname:changes because it now does two things.Why: the dispatched SIMD kernels have never run in an optimized build
The job emulates a Nehalem, and every runtime feature gate in
lance-linalgtests foravx,avx2,fma, or anavx512*feature. A Nehalem has none of them, soSIMD_SUPPORTresolves toNoneand every dispatch takes its fallback arm. That is correct for a SIGILL check, but it means the dispatched kernels are never executed in an optimized build anywhere in CI: the four jobs that run tests all use theciprofile, which inheritsdev.Running the binary that is already on disk closes that gap, and it closes it for free. The bug shape this is aimed at is an out-of-bounds store inside a SIMD kernel that only the optimizer's unrolling makes reachable. Nothing in CI executes those instructions today.
Cost
Nothing beyond the test run itself. Measured locally: after
cargo test --release -p lance-linalg --lib --no-run, changing only the runner env var and running again reports every crateFreshand recompiles nothing. The runner override is not an input to cargo's build fingerprint. The job's existing 60 minute budget already covers a from-scratch release build.What the native run does not cover
Worth stating so the next reader does not assume more than is there.
The binary is built with the job's
RUSTFLAGS: "-C target-cpu=x86-64-v2", and envRUSTFLAGSreplaces the per-targetrustflagsin.cargo/config.tomlwholesale. So#[cfg(target_feature = "avx2")]code is compiled out of it, and only the runtime-dispatched kernels are reachable, since#[target_feature(enable = "avx2")]is additive per function. Both runs use default features, sofp16kernelsis off.Which dispatch tier the native run takes depends on the runner's CPU, and GitHub does not contract a CPU model.
test_x86_runtime_feature_reportwrites the detected features to stderr, so the job log records what actually happened rather than leaving it to be inferred.Why the new step sits before the QEMU build
A step runs only if the ones before it succeeded, and the QEMU steps include building QEMU 8.2.10 from source on a cache miss. With the build and the native run first, a QEMU failure cannot take the native signal down with it. Nothing in the QEMU setup is a prerequisite for the cargo build:
--no-runnever invokes the runner, andlance-linalg'sbuild.rsneeds only a C compiler.The move does not disturb the two ordering constraints in the job.
Swatinem/rust-cachestill precedes the first cargo invocation, andRestore QEMU 8.2.10still precedes both steps that read itscache-hitoutput.The rename, and the one line of Rust
The job id
qemu-pre-haswellis unchanged, so therust-cachekey, which is derived fromgithub.job, is unaffected. Only the display name changes.rust/lance-linalg/src/distance/dot_f16.rsreferred to the job by its display name, which the rename would have left pointing at a string that exists nowhere in the repository. It now names the job id, matching whatrust/lance-linalg/src/simd/i32.rsalready does.Test plan
cargo test --release -p lance-linalg --lib: 389 passed on aarch64-apple-darwin, and 448 passed on x86_64-apple-darwin withRUSTFLAGS="-C target-cpu=x86-64-v2"and the pass-through runner. That second run is the closest local equivalent of the x86 leg, and it confirms three things at once: the v2 baseline builds,envworks as a cargo runner, andtest_x86_runtime_feature_reportis visible without--nocapturecargo fmt --all -- --check,cargo clippy -p lance-linalg --all-targets -- -D warnings, andRUSTDOCFLAGS="-D warnings" cargo doc -p lance-linalg --no-deps --document-private-items: cleanThe AVX2 and AVX-512 arms were compiled but not executed. This machine is aarch64, and an x86_64 build under Rosetta reports neither feature, so those arms first execute when this job runs on a real x86 runner. That is the change.
Two gaps this leaves open
lance-linalgin an optimized profile with--features fp16kernels. Turning it on compiles C, which would cost a rebuild and undo the zero-cost property above, so it belongs in its own change.cargo/config.tomlis not in this workflow'spaths:filter, so a PR that changes[profile.release]or the x86 baseline runs no Rust CI at all. One line fixes it, but it also makes more PRs run the full Rust suite, which is a runner budget call for a maintainer rather than a drive-by here