fix(build): support pre-Haswell x86_64 hosts - #8377
Conversation
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The x86-64-v2 plus runtime-dispatch direction is appropriate, but this revision does not reach the published Python wheel path and it materially slows the PQ-sized batch kernels used on modern CPUs. Apply the compatible baseline to every release-relevant Cargo configuration, exercise that exact build under a pre-AVX2 CPU, and remove or explicitly resolve the measured batch regression before changing the default.
| rustflags = ["-C", "target-cpu=haswell", "-C", "target-feature=+avx2,+fma,+f16c"] | ||
| # Keep the workspace baseline below AVX so binaries can load before runtime | ||
| # feature detection selects optimized SIMD kernels. | ||
| rustflags = ["-C", "target-cpu=x86-64-v2"] |
There was a problem hiding this comment.
The Python release build still permits Haswell/AVX2 instructions, so this root-only baseline cannot fix the reported pre-AVX2 import path. python/.cargo/config.toml still adds target-cpu=haswell and +avx2,+fma,+f16c, while both x86_64 wheel branches run Cargo with working-directory: python.
Reproducer
On this head, from python/:
cargo rustc --locked --manifest-path ../rust/lance-core/Cargo.toml -p lance-core --lib -- --print cfgThe compatible build should omit AVX/FMA compile-time features; the output still contained target_feature="avx", target_feature="avx2", target_feature="f16c", and target_feature="fma".
Please lower or remove the nested Python override too and make the pre-Haswell regression execute from the Python/wheel build context, so the actual release artifact is covered.
| // path (base-equivalent). The runtime-dispatch path below is only | ||
| // compiled/reached when the baseline is below AVX2 (pre-Haswell builds). | ||
| // On a build whose baseline already guarantees AVX2, avoid the | ||
| // per-vector runtime dispatch + `#[target_feature]` wrapping that taxes |
There was a problem hiding this comment.
Switching the default below AVX2 makes normal root builds take the eager runtime-dispatched batch path that this comment describes as taxed, and the measured cost is large at the dimension-8 PQ size. Two runs against this head measured Dot at 2.1–2.3× slower, L2 at 2.1–2.2× slower, and cosine at 2.4–2.6× slower than the current Haswell baseline.
Reproducer
I used a release harness with fat LTO and one codegen unit. It filled an 8-element key and 1,048,576-element batch with deterministic values, then timed these public calls for two seconds each:
black_box(<f32 as Dot>::dot_batch(&key, &batch, 8).sum::<f32>());
black_box(<f32 as L2>::l2_batch(&key, &batch, 8).sum::<f32>());
black_box(<f32 as Cosine>::cosine_batch(&key, &batch, 8).sum::<f32>());The same source was compiled and run with the exact old and new flags:
RUSTFLAGS='-C target-cpu=x86-64-v2' cargo run --release
RUSTFLAGS='-C target-cpu=haswell -C target-feature=+avx2,+fma,+f16c' cargo run --releaseOne repeat produced 3.45 vs 1.52 ns/vector for Dot, 3.38 vs 1.51 for L2, and 6.51 vs 2.53 for cosine.
Please keep the compatibility goal but avoid this eager/taxed path for the hot small dimensions (or establish another benchmark-backed performance design), and add a representative regression benchmark before changing the default.
There was a problem hiding this comment.
Addressed in 5762b17. Dimension-8 Dot and L2 now use allocation-free runtime-selected folds, cosine uses an inline SSE iterator, and the new batch_distance benchmark exercises the requested public calls over 1,048,576 values. Paired results remove the reported 2.1–2.6× regression (x86-64-v2: ~147/167/350 µs; Haswell: ~162/170/331 µs for Dot/L2/cosine).
|
Blocked: the requested Python/wheel-context Nehalem CI update cannot be published because the repository-scoped lance-gatefixer App lacks permission to modify Remote head The smallest unblock is to grant the App workflow permission and re-wake this FixAgent. Alternatively, a maintainer can update |
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The portable baseline and runtime-dispatch direction is appropriate, and the earlier modern-CPU batch regression is substantially resolved. The release artifact that originally failed still needs direct pre-AVX2 coverage before this becomes the default. Extend the existing Nehalem job to build and import the Python extension using its actual default Cargo configuration.
| rustflags = ["-C", "target-cpu=haswell", "-C", "target-feature=+avx2,+fma,+f16c"] | ||
| # Published extensions must load before runtime SIMD detection selects an | ||
| # optimized kernel, including on x86_64 hosts without AVX or AVX2. | ||
| rustflags = ["-C", "target-cpu=x86-64-v2"] |
There was a problem hiding this comment.
This lowers the Python wheel baseline, but the regression suite still does not build and import that extension on the CPU class where the published import failure occurs. The existing qemu-pre-haswell job overrides RUSTFLAGS and runs only lance-linalg, so it can pass without exercising this nested config or Python initialization. Update that job to build the release extension from python/ with the default config and import it under qemu-x86_64 -cpu Nehalem, removing the override that masks both changed configs. Until that artifact executes there, the primary no-SIGILL claim is inferred from compiler flags rather than verified.
|
Blocked: review comment 3737532679 requests a Python-extension Nehalem workflow change that the repository-scoped lance-gatefixer App is not permitted to publish. At verified remote head The smallest unblock is to grant the App workflow-write permission and re-wake this FixAgent. Alternatively, a maintainer can apply the requested |
Summary
Root cause
The workspace and nested Python Cargo configurations enabled AVX2, FMA, and F16C globally, so LLVM could emit those instructions before runtime SIMD detection ran. Binaries therefore trapped during import on x86_64 hosts without AVX2. The compatible baseline keeps startup code portable while runtime dispatch selects optimized kernels on newer hosts.
The first sub-AVX2 batch implementation also materialized complete result batches behind target-feature call boundaries. That dominated PQ-sized dimension-8 workloads. The revised paths select the SIMD tier once, use specialized allocation-free folds for Dot and L2, and use an inline SSE iterator for dimension-8 cosine.
Validation
cargo test -p lance-linalg --lib(199 passed)cargo fmt --allcargo clippy --all --tests --benches -- -D warningsuv run make buildfrompython/with a clean target directoryuv run make lintfrompython/Fixes #6618