Repository navigation
ci: resolve target-cpu=native per runner and key the cache to it - #1526
Conversation
.cargo/config.toml builds with target-cpu=native, and cargo fingerprints the flag text, not the CPU. dir-sim-persist.yml and identity.yml set no RUSTFLAGS and restore target/ from Swatinem/rust-cache, so an artifact built on a runner with AVX-512 was reused on one without: the lance-graph-contract build script died with SIGILL (dir-sim log in Lance, red on main since 10325b3). Both workflows now hash /proc/cpuinfo's flags into the cache key. Also corrects the config comment that claimed every workflow overrides native with RUSTFLAGS; only build, rust-test and style do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011BMJnDBAfzcuQh5HeT39Rh
|
Warning Review limit reachedYour organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Next included review available in 49 minutes. View limit details
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
A step resolves native to the CPU name and exact target-feature list (.github/scripts/resolve-native-rustflags.sh) and sets RUSTFLAGS. Cargo then sees different flag text on a different CPU and recompiles by itself instead of reusing an artifact that dies with SIGILL. The cache key is derived from the same flags, replacing the /proc/cpuinfo hash. Checked locally: the resolved flags give the same cfg as native; the same CPU rebuilds nothing; another CPU's flags rebuild 16 crates of lance-graph-contract; clippy -D warnings passes with them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011BMJnDBAfzcuQh5HeT39Rh
AdaWorldAPI
changed the title
Each resolve step writes the CPU name and feature hash it resolved native to into $GITHUB_STEP_SUMMARY, so every run shows which runner CPU the job landed on without a tracked list. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011BMJnDBAfzcuQh5HeT39Rh
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d30832a99c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| features=$(rustc --print cfg -C target-cpu=native \ | ||
| | sed -n 's/^target_feature="\(.*\)"$/+\1/p' | sort | paste -sd, -) | ||
| [ -n "$features" ] || { echo "resolve-native-rustflags: no target features reported" >&2; exit 1; } | ||
| echo "-Ctarget-cpu=$cpu -Ctarget-feature=$features${*:+ $*}" |
There was a problem hiding this comment.
On a VM that masks AVX-512 while retaining a Skylake server CPU model, these flags are not equivalent to target-cpu=native: the named target-cpu enables its model defaults, while this generated target-feature value contains only +feature entries and therefore cannot disable features hidden by the hypervisor. With the pinned rustc 1.99.0, rustc -C help identifies this option as “target specific attributes,” and rustc --print cfg -Ctarget-cpu=skylake-avx512 -Ctarget-feature=+sse2 still reports target_feature="avx512f"; thus the identity and dir-sim workflows can compile AVX-512 instructions and then SIGILL on precisely the masked runner this change intends to support. Compare the named CPU's defaults with the native feature set and emit negative overrides such as -avx512f for missing features.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and reproduced here: rustc --print cfg -Ctarget-cpu=skylake-avx512 -Ctarget-feature=+sse2 still reports avx512f. Fixed in 8342c9f. The flags no longer name a CPU. They are now the target's baseline CPU plus the exact native feature list, and the script fails unless rustc --print cfg under those flags matches the native set exactly. Disable run: putting the CPU name back (sapphirerapids, on a cascadelake host) makes that check fail and list the 16 features the name adds.
Generated by Claude Code
A named target-cpu enables its model's default features, and a +feature list cannot take them back: rustc --print cfg -Ctarget-cpu=skylake-avx512 -Ctarget-feature=+sse2 still reports avx512f. On a VM that hides AVX-512 from a CPU that keeps its model name, the resolved flags compiled AVX-512 back in, the SIGILL this script exists to prevent. The flags are now the target's baseline CPU plus the exact native feature list, and the script checks that rustc --print cfg under those flags equals the native set, failing otherwise. Disable run: with the CPU name put back (sapphirerapids on this cascadelake host) the check fails and lists the 16 features the name adds. --cpu prints the native CPU name for the job summary. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011BMJnDBAfzcuQh5HeT39Rh
The four ci commits ported here from #1526 are reverted so this PR carries only the NAL work and the CI change stays visible in its own PR. #1526 merges first; this branch then picks it up from main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011BMJnDBAfzcuQh5HeT39Rh
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011BMJnDBAfzcuQh5HeT39Rh
2 participants
dir-sim log in Lancehas been red onmainsince 10325b3, and on any PR whose job lands on the wrong runner. Thelance-graph-contractbuild script dies with SIGILL before anything compiles.Cause
.cargo/config.tomlbuilds with-Ctarget-cpu=native.dir-sim-persist.ymlandidentity.ymlset noRUSTFLAGSand restoretarget/withSwatinem/rust-cache.The same thing happened locally when this container moved from a host with avx512fp16/bf16 to one that
rustcreports ascascadelake(32 instead of 44 target features).Change
.github/scripts/resolve-native-rustflags.shwritesnativeout as the exact feature list fromrustc --print cfg -C target-cpu=native. The output is only-Ctarget-feature=+…, with no-Ctarget-cpu=<name>.+featurelist cannot turn them off.rustc --print cfg -Ctarget-cpu=skylake-avx512 -Ctarget-feature=+sse2still reportsavx512f. On a VM that hides AVX-512 from a CPU that keeps its model name, the earlier version compiled AVX-512 back in. The baseline CPU plus the exact list has no such hidden defaults.rustc --print cfgunder the produced flags equals the native set exactly.--cpuprints the namenativeresolves to, for logs.dir-sim-persist.ymlandidentity.yml, a step runs the script, setsRUSTFLAGS, and writes the CPU name and the flags hash to the job summary. A runner with a different CPU produces different flags, and Cargo recompiles on its own.keyis derived from the same flags, so the key and the build cannot disagree..cargo/config.tomlsets onlytarget-cpu, soRUSTFLAGSreplacing it loses nothing.build.yml,rust-test.ymlandstyle.ymlalready set-C target-cpu=x86-64-v3; their artifacts do not depend on the CPU..cargo/config.tomlthat claimed every workflow overridesnative.Companion: build-time drift warning in ndarray (AdaWorldAPI/ndarray#360)
This PR stops a cached artifact from reaching a CPU it was not built for. The SIGILL guard has a second gap, and it sits in ndarray.
ndarray::cpu_guardcompares the running CPU only against the features that have a row in its table (cfg!(target_feature = …)per row). A feature the build enables but the table lacks is never checked. The guard passes and the binary can still die with SIGILL. This happens when a toolchain bump lets LLVM enable a feature the table predates.build.rsthat readsCARGO_CFG_TARGET_FEATURE, which only build scripts see, and emitscargo:warningfor every enabled feature without a guard row. Thesse/sse2/x87baseline is excluded. The parser and filter live insrc/cpu_guard_drift.rs, which bothbuild.rsand three unit tests include, so the tests exercise the code the build runs.target-cpu=sapphirerapids):cpu_guard has no row for enabled target features: amx-bf16, amx-int8, amx-tile, clflushopt, ermsb, lahfsahf, movdir64b, movdiri, prfchw.Verification (local)
The flags reproduce native exactly (
rustc --print cfgis identical), and compiling with them gives no rustc warnings.Disable run: putting the CPU name back (
-Ctarget-cpu=sapphirerapidson this cascadelake host) makes the self-check fail and list the 16 features the name adds.Rebuild behaviour of
lance-graph-contractin a fresh target dir (measured with the first version of the flags):cargo clippy -p lance-graph-contract --all-targets -- -D warningspasses with the resolved flags.Both workflow files parse as YAML; the resolve step runs after the toolchain setup and before the cache.
Not yet verified on a runner: whether runners with different CPUs really get separate caches only shows once several runs have landed.
This PR is the only place the CI change lives. It was briefly ported into #1525 and taken back out (5b37379), so the NAL PR stays NAL-only. #1525 picks it up from
mainafter this merges.🤖 Generated with Claude Code
https://claude.ai/code/session_011BMJnDBAfzcuQh5HeT39Rh
Generated by Claude Code