-
-
Notifications
You must be signed in to change notification settings - Fork 161
perf(codegen): let a[i] += 1 reach the loop tier a[i] = a[i] + 1 already had — 277 to 25.5 instructions (#10743) #10752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
proggeramlug
wants to merge
5
commits into
PerryTS:main
from
proggeramlug:perf/10743-compound-alias-fold
Closed
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
efa2050
perf(codegen,hir): stop re-proving a loop-invariant array receiver on…
perry-bot 1246fd9
changelog: fragment for #10718
perry-bot 8d46937
Merge remote-tracking branch 'origin/main' into storebase
perry-bot 2b2b890
perf(codegen): hoist the loop-invariant receiver proof for array elem…
perry-bot 4c0a29b
perf(codegen): let `a[i] += 1` reach the loop tier `a[i] = a[i] + 1` …
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| **Indexed reads on an ordinary `Array` no longer re-prove a loop-invariant receiver on every element.** | ||
|
|
||
| An indexed read cost **87 instructions per element** — against 6 for the same arithmetic on a `Float64Array` and 16 for node — and none of it was a runtime call. 56 of the 87 were loop-invariant receiver revalidation re-executed every iteration: the NaN-box tag and handle-band test, the forwarding-flag follow, and a six-load live-head guard. | ||
|
|
||
| perry already had tiers that hoist that proof into the loop preheader. They were declining at one gate, `array_static_type_excluded` — a *declared static type* test in front of a tier that is otherwise fully runtime-guarded — so `const a: number[]` got it and plain `new Array(400)`, which infers `Array<any>`, did not. Ordinary JavaScript never reached the tier it already had. | ||
|
|
||
| Separately, `a[i] += 1` cost **948** instructions per element, 3.7× the identical `a[i] = a[i] + 1`, and no annotation helped: the compound-assignment spill temporaries were minted as `Type::Any`, erasing the receiver's array-ness and the index's integer-ness before codegen saw the statement. | ||
|
|
||
| Array read **87 → 13.5** (node 16.3), `a[i] += 1` **948 → 273**, `a[i] += b[i]` **1025 → 347**. A particle simulation over four numeric arrays spends **60.9% fewer instructions** and **59% less peak RSS**. The bare loop and both `Float64Array` paths are unchanged to the instruction. | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| **Stores to an ordinary `Array` element no longer re-prove a loop-invariant receiver on every element.** | ||
|
|
||
| An indexed write cost **105 instructions per element** — against 8 for the same store to a `Float64Array` and 12 for node — with zero runtime calls. **51 of the 105 were loop-invariant** receiver revalidation, and a further 42 was a write-barrier decision provable away from the value's type. | ||
|
|
||
| This widens the store admission the way #10731 widened reads. The gate was `has_materialization_hazard`, which a trailing `console.log` is enough to set. | ||
|
|
||
| `a[i] = k + i` **105 → 17.4**, `a[i] = a[i] + 1` **256 → 24.5** (node 18.7), `a[i] = a[i] + b[i]` **333 → 35.9**. The bare loop, both `Float64Array` paths and the indexed read are unchanged to the instruction. | ||
|
|
||
| Note this moves none of the five real programs in #10695 — their loop bodies are multi-statement or contain calls, which no current tier admits (#10741) — and `a[i] += 1` is unaffected because its lowering is two statements (#10743). |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| **`a[i] += 1` reaches the same loop tier as `a[i] = a[i] + 1`.** | ||
|
|
||
| The two spellings are the same operation and node compiles both to the same cost. perry compiled them **11× apart** — 277 instructions per element against 24 — and the slow one was the idiomatic spelling. | ||
|
|
||
| HIR lowers a compound member assignment into two immutable alias `Let`s plus the store, so the base and the key are each evaluated exactly once and before the right-hand side. The classic range-loop matcher admits exactly ONE statement, so the lowering guaranteed the statement could never reach the tier. Annotating the array changed nothing: the obstacle is the statement count, not type information. | ||
|
|
||
| The temporaries stay. They are load-bearing — an RHS call can reassign the bindings they were read from, and the store must still land at the index evaluated before it ran. Instead the matcher folds them, and only for the guarded fast clones: the slow clone lowers the statements as written, so a failed guard and every side exit still execute the specified evaluation order. Inside the matched subset the fold is exact, because the body walk is a whitelist that admits no call, closure, `await`, update or assignment anywhere in the statement — nothing can write the locals the aliases read. | ||
|
|
||
| `a[i] += 1` **277 → 25.5**, `a[i] -= 1` **208 → 27.5**, `a[i] += b[i]` **347 → 35.9** (identical to `a[i] = a[i] + b[i]`), `a[i] *= 1` **206 → 25.5**, `a[i] |= 0` **236 → 52.5**. The bare loop, both `Float64Array` paths, the indexed read and write, and both expanded spellings are unchanged — their emitted LLVM IR is byte-identical. | ||
|
|
||
| This needs none of #10741's mid-iteration side-exit discipline: the folded-away statements perform no stores, so there is nothing to un-do when a guard fails partway. It also moves none of the five real programs in #10695 — their loop bodies are still multi-statement or contain calls, which no current tier admits. |
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
262 changes: 262 additions & 0 deletions
262
crates/perry-codegen/src/stmt/compound_alias_fold_tests.rs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,262 @@ | ||
| //! #10743: the compound-assignment alias fold, and the shapes it declines. | ||
| //! | ||
| //! `a[i] += 1` is lowered by HIR's `hoist_compound_member_assign` into two | ||
| //! immutable alias `Let`s plus the store, so the base and the key are each | ||
| //! evaluated exactly once and before the right-hand side. The classic | ||
| //! range-loop matcher admits exactly ONE statement, so the idiomatic spelling | ||
| //! could never reach the tier that makes the expanded `a[i] = a[i] + 1` fast: | ||
| //! measured 277 instructions per element against 24 for the expanded form on | ||
| //! the same array, and annotating the array changed nothing, because the | ||
| //! obstacle is the statement count rather than type information. | ||
| //! | ||
| //! The canonical body below is transcribed from a `--print-hir` dump of | ||
| //! `for (let i = 0; i < 400; i++) a[i] += 1;`, not guessed: | ||
| //! | ||
| //! ```text | ||
| //! Let { id: 5, name: "__cmpd_base_5", mutable: false, init: Some(LocalGet(1)) } | ||
| //! Let { id: 6, name: "__cmpd_key_6", mutable: false, init: Some(LocalGet(4)) } | ||
| //! Expr(IndexSet { object: LocalGet(5), index: LocalGet(6), | ||
| //! value: Binary { Add, IndexGet { LocalGet(5), LocalGet(6) }, | ||
| //! Integer(1) } }) | ||
| //! ``` | ||
| //! | ||
| //! Every `declines_*` test here is a guard's witness: it is the test that goes | ||
| //! red when that condition is deleted from the fold. | ||
|
|
||
| #![cfg(test)] | ||
|
|
||
| use perry_hir::types::Type; | ||
| use perry_hir::{BinaryOp, Expr, Stmt}; | ||
|
|
||
| use super::loops::packed_f64_range_loop_compound_alias_fold; | ||
|
|
||
| const ARRAY: u32 = 1; | ||
| const COUNTER: u32 = 4; | ||
| const BASE_TEMP: u32 = 5; | ||
| const KEY_TEMP: u32 = 6; | ||
|
|
||
| fn temp(id: u32, name: &str, mutable: bool, init: Expr) -> Stmt { | ||
| Stmt::Let { | ||
| id, | ||
| name: name.to_string(), | ||
| ty: Type::Number, | ||
| mutable, | ||
| init: Some(init), | ||
| } | ||
| } | ||
|
|
||
| /// `__cmpd_base_5[__cmpd_key_6] = __cmpd_base_5[__cmpd_key_6] + 1` | ||
| fn alias_store() -> Stmt { | ||
| Stmt::Expr(Expr::IndexSet { | ||
| object: Box::new(Expr::LocalGet(BASE_TEMP)), | ||
| index: Box::new(Expr::LocalGet(KEY_TEMP)), | ||
| value: Box::new(Expr::Binary { | ||
| op: BinaryOp::Add, | ||
| left: Box::new(Expr::IndexGet { | ||
| object: Box::new(Expr::LocalGet(BASE_TEMP)), | ||
| index: Box::new(Expr::LocalGet(KEY_TEMP)), | ||
| }), | ||
| right: Box::new(Expr::Integer(1)), | ||
| }), | ||
| }) | ||
| } | ||
|
|
||
| /// What the store must fold to: `a[i] = a[i] + 1`, the shape the tier already | ||
| /// admits and already beats node on. | ||
| fn expanded_store() -> Stmt { | ||
| Stmt::Expr(Expr::IndexSet { | ||
| object: Box::new(Expr::LocalGet(ARRAY)), | ||
| index: Box::new(Expr::LocalGet(COUNTER)), | ||
| value: Box::new(Expr::Binary { | ||
| op: BinaryOp::Add, | ||
| left: Box::new(Expr::IndexGet { | ||
| object: Box::new(Expr::LocalGet(ARRAY)), | ||
| index: Box::new(Expr::LocalGet(COUNTER)), | ||
| }), | ||
| right: Box::new(Expr::Integer(1)), | ||
| }), | ||
| }) | ||
| } | ||
|
|
||
| fn canonical_body() -> Vec<Stmt> { | ||
| vec![ | ||
| temp(BASE_TEMP, "__cmpd_base_5", false, Expr::LocalGet(ARRAY)), | ||
| temp(KEY_TEMP, "__cmpd_key_6", false, Expr::LocalGet(COUNTER)), | ||
| alias_store(), | ||
| ] | ||
| } | ||
|
|
||
| fn debug(stmts: &[Stmt]) -> String { | ||
| format!("{stmts:?}") | ||
| } | ||
|
|
||
| #[test] | ||
| fn folds_the_canonical_compound_assignment_to_the_expanded_store() { | ||
| let folded = | ||
| packed_f64_range_loop_compound_alias_fold(&canonical_body()).expect("shape must fold"); | ||
| assert_eq!( | ||
| debug(&folded), | ||
| debug(std::slice::from_ref(&expanded_store())), | ||
| "the fold must produce exactly the expanded spelling" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn folds_an_arithmetic_key_initialiser() { | ||
| // `a[i * 2 + 1] += 1` spills the whole index expression into the key temp. | ||
| let key = Expr::Binary { | ||
| op: BinaryOp::Add, | ||
| left: Box::new(Expr::Binary { | ||
| op: BinaryOp::Mul, | ||
| left: Box::new(Expr::LocalGet(COUNTER)), | ||
| right: Box::new(Expr::Integer(2)), | ||
| }), | ||
| right: Box::new(Expr::Integer(1)), | ||
| }; | ||
| let body = vec![ | ||
| temp(BASE_TEMP, "__cmpd_base_5", false, Expr::LocalGet(ARRAY)), | ||
| temp(KEY_TEMP, "__cmpd_key_6", false, key.clone()), | ||
| alias_store(), | ||
| ]; | ||
| let folded = packed_f64_range_loop_compound_alias_fold(&body).expect("shape must fold"); | ||
| let text = debug(&folded); | ||
| assert!( | ||
| !text.contains("LocalGet(5)") && !text.contains("LocalGet(6)"), | ||
| "no alias id may survive the fold: {text}" | ||
| ); | ||
| assert!( | ||
| text.contains("Mul"), | ||
| "the key tree must be substituted: {text}" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn declines_a_mutable_alias() { | ||
| // Guard: `mutable: false`. A writable binding is not an alias -- nothing | ||
| // here proves its value at the store is the value it was bound to. | ||
| let mut body = canonical_body(); | ||
| if let Stmt::Let { mutable, .. } = &mut body[0] { | ||
| *mutable = true; | ||
| } | ||
| assert!(packed_f64_range_loop_compound_alias_fold(&body).is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn declines_a_user_named_binding() { | ||
| // Guard: the `__cmpd_` name. The fold's argument rests on these temps | ||
| // being the compiler's own compound-assign spills, read only by the one | ||
| // statement they were minted for. A user `const` in the loop body belongs | ||
| // to the general multi-statement tier (#10741), not here. | ||
| let mut body = canonical_body(); | ||
| if let Stmt::Let { name, .. } = &mut body[0] { | ||
| *name = "userConst".to_string(); | ||
| } | ||
| assert!(packed_f64_range_loop_compound_alias_fold(&body).is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn declines_an_initialiser_outside_the_stable_grammar() { | ||
| // Guard: `packed_f64_range_loop_alias_init_is_stable`. An element read is | ||
| // not re-evaluation-safe the way a local read is -- the folded statement | ||
| // evaluates the key tree twice. | ||
| let mut body = canonical_body(); | ||
| if let Stmt::Let { init, .. } = &mut body[1] { | ||
| *init = Some(Expr::IndexGet { | ||
| object: Box::new(Expr::LocalGet(ARRAY)), | ||
| index: Box::new(Expr::LocalGet(COUNTER)), | ||
| }); | ||
| } | ||
| assert!(packed_f64_range_loop_compound_alias_fold(&body).is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn declines_a_body_longer_than_two_aliases_and_a_store() { | ||
| let mut body = canonical_body(); | ||
| body.insert(0, temp(7, "__cmpd_base_7", false, Expr::LocalGet(ARRAY))); | ||
| assert!(packed_f64_range_loop_compound_alias_fold(&body).is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn declines_a_body_with_no_aliases() { | ||
| // A single statement is already the shape the tier takes; the fold must | ||
| // not claim it, or it would clear and rebuild an access map for nothing. | ||
| assert!( | ||
| packed_f64_range_loop_compound_alias_fold(std::slice::from_ref(&expanded_store())) | ||
| .is_none() | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn declines_a_repeated_alias_id() { | ||
| // Two bindings for one id would make the substitution order-dependent. | ||
| let body = vec![ | ||
| temp(BASE_TEMP, "__cmpd_base_5", false, Expr::LocalGet(ARRAY)), | ||
| temp(BASE_TEMP, "__cmpd_key_5", false, Expr::LocalGet(COUNTER)), | ||
| alias_store(), | ||
| ]; | ||
| assert!(packed_f64_range_loop_compound_alias_fold(&body).is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn declines_when_the_last_statement_is_not_an_expression() { | ||
| let body = vec![ | ||
| temp(BASE_TEMP, "__cmpd_base_5", false, Expr::LocalGet(ARRAY)), | ||
| temp(KEY_TEMP, "__cmpd_key_6", false, Expr::LocalGet(COUNTER)), | ||
| Stmt::Return(Some(Expr::LocalGet(BASE_TEMP))), | ||
| ]; | ||
| assert!(packed_f64_range_loop_compound_alias_fold(&body).is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn declines_an_alias_without_an_initialiser() { | ||
| let body = vec![ | ||
| Stmt::Let { | ||
| id: BASE_TEMP, | ||
| name: "__cmpd_base_5".to_string(), | ||
| ty: Type::Number, | ||
| mutable: false, | ||
| init: None, | ||
| }, | ||
| temp(KEY_TEMP, "__cmpd_key_6", false, Expr::LocalGet(COUNTER)), | ||
| alias_store(), | ||
| ]; | ||
| assert!(packed_f64_range_loop_compound_alias_fold(&body).is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn the_logical_assignment_shape_folds_but_stays_unversionable() { | ||
| // `a[i] ||= 3` spills the same two aliases but ends in `Expr::Logical`, | ||
| // whose right operand is the store. The fold is shape-agnostic, so it | ||
| // rewrites the statement -- and the classic body walk then declines it, | ||
| // because `packed_f64_range_loop_pure_expr_collect` has no `IndexSet` arm. | ||
| // This test pins the second half of that sentence: if a future widening | ||
| // admits `Logical`, the short-circuit semantics have to be re-argued. | ||
| let body = vec![ | ||
| temp(BASE_TEMP, "__cmpd_base_5", false, Expr::LocalGet(ARRAY)), | ||
| temp(KEY_TEMP, "__cmpd_key_6", false, Expr::LocalGet(COUNTER)), | ||
| Stmt::Expr(Expr::Logical { | ||
| op: perry_hir::LogicalOp::Or, | ||
| left: Box::new(Expr::IndexGet { | ||
| object: Box::new(Expr::LocalGet(BASE_TEMP)), | ||
| index: Box::new(Expr::LocalGet(KEY_TEMP)), | ||
| }), | ||
| right: Box::new(Expr::IndexSet { | ||
| object: Box::new(Expr::LocalGet(BASE_TEMP)), | ||
| index: Box::new(Expr::LocalGet(KEY_TEMP)), | ||
| value: Box::new(Expr::Integer(3)), | ||
| }), | ||
| }), | ||
| ]; | ||
| let folded = packed_f64_range_loop_compound_alias_fold(&body).expect("shape folds"); | ||
| let mut accesses = std::collections::BTreeMap::new(); | ||
| assert!( | ||
| !super::loops::packed_f64_range_loop_body_collect( | ||
| &folded, | ||
| COUNTER, | ||
| None, | ||
| &mut accesses, | ||
| None, | ||
| ), | ||
| "a logical compound assignment must not be admitted by the classic walk" | ||
| ); | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 19859
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 16929
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 5754
Remove the stale compound-assignment claims from the
10718fragments.The release script concatenates every numeric
changelog.d/fragment present at the release SHA. The10718fragments therefore publish intermediate compound-assignment behavior alongside10743. Remove the compound-assignment paragraph and metrics from10718-array-index-hoist.md, and remove the sentence stating thata[i] += 1is unaffected from10718-array-store-hoist.md. Keep the indexed-read and store-hoisting claims.🤖 Prompt for AI Agents