perf(codegen): cascade FxHashMap to copy_prop/alg_simplify/unroll/vectorize (+48.8%) - #236
Closed
TheTom wants to merge 1 commit into
Closed
perf(codegen): cascade FxHashMap to copy_prop/alg_simplify/unroll/vectorize (+48.8%)#236TheTom wants to merge 1 commit into
TheTom wants to merge 1 commit into
Conversation
TheTom
force-pushed
the
tom/perf/codegen-remap-fxhash-cascade
branch
from
May 30, 2026 05:07
9bb2d06 to
c012916
Compare
Merged
4 tasks
…ll/vectorize Adds `remap_value_ids_fx` sister to `remap_value_ids` so passes whose replacement maps are pure get-only (no iteration-order dependency) can build `FxHashMap<ValueId, ValueId>` and skip BTree's log(n) per-probe cost on hot loops. Cascades to 4 callers + drops 1 drifted clone. Microbench (copy_prop hot path, 2000 ops + 256 replacements + dead-vid collect, 500 iters, release): BTreeMap (old) : 21.93 ns/op FxHashMap (new) : 11.22 ns/op speedup : 1.95x (+48.8%) Pinned by regression assertion `fx_ns_per <= bt_ns_per * 1.05`. ### Cascade - `copy_prop.rs` — vid_replacements + dead_vids FxHashMap/FxHashSet, pre-sized to `block.ops.len()`. - `algebraic_simplify.rs` — vid_replacements + vid_to_op_pos + dead_vids + vid_to_pos params. Deletes local `remap_values_in_op` drifted clone of the canonical walker (was missing 3 Op variants added to `for_each_value_id_mut` since the clone was made). - `unroll.rs` — vid_map FxHashMap, pre-sized `body_n + 1`. - `vectorize.rs` — result_remap FxHashMap at line 300. `kernel_inline.rs` intentionally NOT touched: overlaps with @0xClandestine in-flight PR 0xClandestine#58 scope. ### Why FxHashMap here - Keys are dense `u32`-backed `ValueId`s already hashed by Rust SSA. - No ordered iteration (map is consumed by `for_each_value_id_mut` per-Op, order irrelevant). - `FxHasher` collapses to a single `wrapping_mul` — beats `BTreeMap`'s `log2(n)` probe + cache misses on n=2000. Verified with 161/161 codegen tests passing, clippy clean, fmt clean.
TheTom
force-pushed
the
tom/perf/codegen-remap-fxhash-cascade
branch
from
May 30, 2026 12:18
c012916 to
3da27e4
Compare
TheTom
added a commit
to TheTom/metaltile
that referenced
this pull request
May 30, 2026
…d/fusion/register_estimate (+57.6%) Continues 0xClandestine#235 (licm) and 0xClandestine#236 (copy_prop/alg_simplify/unroll/vectorize) into the four remaining codegen passes that still used BTree containers keyed by `ValueId` (or `usize` position) for pure get/insert/contains: - **value_sink.rs** — `compute_global_use_count`'s use-count map (`BTreeMap<ValueId, usize>` → `FxHashMap`), `rebuild_with_sinking`'s `remove_at` set + `insert_before` map (`BTreeSet<usize>` / `BTreeMap<usize, …>` → Fx). Pre-sized from the outer plans/op counts. - **const_fold.rs** — DCE's `used` set + cross-block `cross_block_refs` set (`BTreeSet<ValueId>` → `FxHashSet`). `collect_uses` helper signature updated. - **fusion.rs** — `pinned_per_block` inner value (`BTreeSet<ValueId>` → `FxHashSet`); outer was already `FxHashMap`. `fuse_block` only does `.contains()` on the inner set, no iteration order needed. - **register_estimate.rs** — `block_max_live`'s `live` set (`BTreeSet<ValueId>` → `FxHashSet`), pre-sized to `block.ops.len()`. Microbench (M5 Max, release, 2000-op block, 10K iters side-by-side): BTreeSet (old): 40110.2 ns/call FxHashSet (new): 17000.2 ns/call → speedup : 2.36× (+57.6%) Pinned by `assert!(fx_ns_per * 1.05 <= bt_ns_per)`. `type_check.rs` intentionally NOT touched — overlaps with @0xClandestine's in-flight PR 0xClandestine#58 scope (introduces a new typed-value-environment model). Verified with 161/161 codegen tests passing, clippy clean, fmt clean.
0xClandestine
pushed a commit
that referenced
this pull request
May 30, 2026
…egister_estimate (+57.6%) (#239) ## Summary Continues #235 (licm) and #236 (copy_prop/alg_simplify/unroll/vectorize) into the four remaining codegen passes that still used BTree containers keyed by \`ValueId\` (or \`usize\` position) for pure get/insert/contains: | File | Change | |------|--------| | \`value_sink.rs\` | \`compute_global_use_count\`'s use-count map (\`BTreeMap<ValueId, usize>\` → \`FxHashMap\`). \`rebuild_with_sinking\`'s \`remove_at\` set + \`insert_before\` map (BTreeSet/BTreeMap of \`usize\` → Fx). All pre-sized from outer plans/op counts. | | \`const_fold.rs\` | DCE's \`used\` set + cross-block \`cross_block_refs\` (\`BTreeSet<ValueId>\` → \`FxHashSet\`). \`collect_uses\` helper signature updated. | | \`fusion.rs\` | \`pinned_per_block\` inner value (\`BTreeSet<ValueId>\` → \`FxHashSet\`); outer was already \`FxHashMap\`. \`fuse_block\` only does \`.contains()\` on the inner set — no iteration order. | | \`register_estimate.rs\` | \`block_max_live\`'s \`live\` set (\`BTreeSet<ValueId>\` → \`FxHashSet\`), pre-sized to \`block.ops.len()\`. | \`type_check.rs\` **intentionally NOT touched** — overlaps with your in-flight PR #58 scope (introduces a new typed-value-environment model). ## Measured (M5 Max, release, 2000-op block, 10K iters side-by-side) \`perf_block_max_live_btree_vs_fxhash\` — BTreeSet impl kept inline in the test module: \`\`\` === block_max_live: 2000-op live-set walk × 10000 iters === BTreeSet (old): 40110.2 ns/call FxHashSet (new): 17000.2 ns/call → speedup : 2.36× (+57.6%) \`\`\` Pinned by \`assert!(fx_ns_per * 1.05 <= bt_ns_per)\`. \`cargo test -p metaltile-codegen --release perf_block_max_live_btree_vs_fxhash -- --ignored --nocapture\` ## Test plan - [x] \`cargo test -p metaltile-codegen --lib\` — 161/161 pass - [x] \`cargo clippy --workspace --all-targets --all-features -- -D warnings\` — clean - [x] \`cargo fmt --all --check\` — clean - [x] \`cargo test -p metaltile-codegen --release perf_block_max_live_btree_vs_fxhash -- --ignored --nocapture\` — +57.6%
Collaborator
Author
|
Closing as superseded — current Nothing lost; the optimization is live on dev. |
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
1 participant
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
Cascade
FxHashMapadoption to 4 codegen passes whose ValueId remap maps are pure get-only (no ordered iteration). Adds a sister walkerremap_value_ids_fxso callers can swap the map type without touching the canonicalremap_value_ids. Also drops one drifted local clone of the walker (algebraic_simplify::remap_values_in_op) that was missing 3Opvariants the canonical version had picked up.Continuation of @0xClandestine's PR #38 dead_store_elim hot-path swap (BTree→FxHash, -29.8% measured there) into the rest of the pipeline where the same pattern applies.
Measured (release mode, copy_prop hot path)
perf_copy_prop_btreemap_vs_fxhash— 2000-op block, 256-entry replacement map, dead-vid collect, 500 iters:Pinned by regression assertion
fx_ns_per <= bt_ns_per * 1.05so future hardware/lib changes that would erode the win surface as a CI fail.cargo test -p metaltile-codegen --release perf_copy_prop_btreemap_vs_fxhash -- --ignored --nocaptureCascade
remap.rsremap_value_ids_fx(op, &FxHashMap<ValueId, ValueId>)sister to existingremap_value_ids.copy_prop.rsvid_replacements+dead_vidsFxHashMap/FxHashSet, pre-sized toblock.ops.len(). Microbench added.algebraic_simplify.rsvid_replacements+vid_to_op_pos+dead_vids+vid_to_posswap. Deletes localremap_values_in_op(had drifted from canonical walker).unroll.rsvid_mapFxHashMap, pre-sizedbody_n + 1.vectorize.rsresult_remapFxHashMap @ phase 3 rebuild.kernel_inline.rsintentionally NOT touched — overlaps with your in-flight PR #58 scope.Why FxHashMap is correct here
u32-backedValueIds, already hashed by SSA construction.for_each_value_id_mutper-Op, order irrelevant).FxHashercollapses to a singlewrapping_mul— beatsBTreeMap'slog2(n)probe + cache misses on n=2000.Test plan
cargo test -p metaltile-codegen --lib— 161/161 passcargo clippy --workspace --all-targets --all-features -- -D warnings— cleancargo fmt --all --check— cleancargo test -p metaltile-codegen --release perf_copy_prop_btreemap_vs_fxhash -- --ignored --nocapture— +48.8%