Repository navigation
Conversation
gxhash only compiles with hardware AES enabled at compile time, so any crate depending on ppvm failed to build on x86_64 and aarch64 Linux unless its users set RUSTFLAGS themselves (the repo's .cargo/config.toml only applies to builds inside this repo). Gate the gxhash dependency and every use of it on `all(target_feature = "aes", any(target_arch = "x86_64", target_arch = "aarch64"))` instead of `not(wasm32)`. Without AES the gxhash configs are dropped and ppvm-tableau-sum's fingerprint uses fxhash, as on wasm. This also fixes the dashmap gxhash configs failing to compile with the `gxhash` feature off. Update the build docs and the usage skill. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The example’s guard still permits references to unavailable gxhash configs when AES is enabled but the gxhash feature is disabled.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Makes downstream Rust builds work without AES flags, supporting the publishing work in #237.
Changes:
- Gates gxhash dependencies and configs on supported AES-enabled targets.
- Extends the tableau fingerprint’s fxhash fallback and guards optional gxhash usage.
- Updates installation and contributor guidance.
| File | Description |
|---|---|
| skills/ppvm-usage/SKILL.md | Explains automatic fallback. |
| docs/src/pages/index.astro | Updates installation flags. |
| docs/src/pages/develop.astro | Documents AES gating and fallback. |
| crates/ppvm-traits/src/traits/hash.rs | Gates gxhash implementation and test. |
| crates/ppvm-traits/Cargo.toml | Gates gxhash dependency. |
| crates/ppvm-tableau-sum/src/storage/mod.rs | Extends fingerprint fallback. |
| crates/ppvm-tableau-sum/Cargo.toml | Gates gxhash dependency. |
| crates/ppvm-pauli-word/Cargo.toml | Gates gxhash dependency. |
| crates/ppvm-pauli-sum/src/config/mod.rs | Gates gxhash config module. |
| crates/ppvm-pauli-sum/src/config/indexmap.rs | Gates gxhash configs. |
| crates/ppvm-pauli-sum/src/config/dashmap.rs | Adds gxhash config guards. |
| crates/ppvm-pauli-sum/examples/trotter_qubit_sweep.rs | Guards gxhash benchmark branch. |
| crates/ppvm-pauli-sum/Cargo.toml | Gates gxhash dependency. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
The gxhash configs need both the `gxhash` feature and AES, but the example only checked for AES, so `--no-default-features` builds on AES targets failed to compile it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This branch has not been deployed
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
gxhash only compiles when AES is enabled at compile time. The repo's
.cargo/config.tomlenables it for builds inside this repo, but that config doesn't apply to crates that depend on ppvm. So any crate depending onppvm-pauli-sum,ppvm-tableau-sumand so on failed to build on x86_64 and aarch64 Linux unless its users setRUSTFLAGSthemselves. That blocks publishing to crates.io (#237).ppvm-tableau-sumuses gxhash unconditionally, so turning the feature off wasn't a workaround.This extends the existing wasm32 fallback. The gxhash dependency and every use of it are now gated on
all(target_feature = "aes", any(target_arch = "x86_64", target_arch = "aarch64"))instead of "not wasm32":ppvm-tableau-sumuses fxhash, as on wasm.RUSTFLAGS="-C target-feature=+aes"), nothing changes.ByteGxHashconfigs were never gated, so--no-default-features --features dashmapfailed to compile. It now builds.No simulator or gate code changes. The edits are 13
cfgattributes on the hashing configs, the hashing hook and the fingerprint function, plus the fourCargo.tomldependency tables.Test plan
ppvm-pauli-sum+ppvm-tableau-sumbuilds on x86_64, aarch64 Linux and Apple Silicon (previously failed on the first two), and uses gxhash with+aes.cargo test --workspace(gxhash path): 1141 passed.-C target-feature=-aes): 678 passed across traits, pauli-word, pauli-sum, tableau-sum, tableau and stim, doctests included.cargo clippy --workspace --all-targets -D warnings, the x86_64 all-targets build, the wasm32 CI build,--no-default-features --features dashmap.The benches still use the gxhash configs unguarded. They're only built by
cargo bench/--all-targets, which CI runs on x86_64 with AES enabled.Follow-up for #237: with this merged, the linux-aarch64 wheel no longer needs the
+aes,+neonflag that #237 adds. Removing it makes that wheel portable to CPUs without AES (e.g. Raspberry Pi 4), at some speed cost on AES-capable ARM servers.🤖 Generated with Claude Code