diff --git a/changelog.d/11860-regfix-region-value-reads.md b/changelog.d/11860-regfix-region-value-reads.md new file mode 100644 index 0000000000..82da205fde --- /dev/null +++ b/changelog.d/11860-regfix-region-value-reads.md @@ -0,0 +1,6 @@ +A loop region whose body calls out no longer takes an array the loop only reads +element values from (`const o = xs[i & 63]; o.m()`). Such an array gains one +cheaper load per read, and the call re-checks the region's guard on every +iteration, so the versioned loop was slower than the plain one: #10594 +`userplain` 167 -> 149 and #10510 `date_getTime` 176 -> 158 instructions per +operation. A call-free body keeps these arrays. diff --git a/crates/perry-codegen/src/expr/region_array_loop_tests.rs b/crates/perry-codegen/src/expr/region_array_loop_tests.rs index e99fd4ed54..da3c15fca0 100644 --- a/crates/perry-codegen/src/expr/region_array_loop_tests.rs +++ b/crates/perry-codegen/src/expr/region_array_loop_tests.rs @@ -425,3 +425,86 @@ fn a_store_of_a_value_not_proven_a_number_is_not_bare() { "the string store must take today's store:\n{f_text}" ); } + +/// `a[i & 63]`, a static index in `[0, 63]`. +fn masked(arr: u32) -> Expr { + Expr::IndexGet { + object: Box::new(Expr::LocalGet(arr)), + index: Box::new(Expr::Binary { + op: BinaryOp::BitAnd, + left: Box::new(Expr::LocalGet(I)), + right: Box::new(Expr::Integer(63)), + }), + } +} + +/// An array the loop only reads element VALUES from (`const o = a[i & 63]; +/// f(o)`) in a body that calls out is no region array: the region would save +/// one load per read and re-check its guard on every iteration. A read a +/// Number consumer takes makes it one, and so does the value read in a body +/// that runs no JS (`const o = a[i & 63]; o.d = i`), where the facts hold +/// across iterations. +#[test] +fn an_array_only_read_for_element_values_is_not_a_region_array() { + let call = |arg: Expr| { + Stmt::Expr(Expr::Call { + callee: Box::new(Expr::LocalGet(F)), + args: vec![arg], + type_args: Vec::new(), + byte_offset: 0, + }) + }; + const O: u32 = 10; + let body = vec![ + Stmt::Let { + id: O, + name: "o".to_string(), + ty: Type::Any, + mutable: false, + init: Some(masked(A)), + }, + call(Expr::LocalGet(O)), + ]; + let ir = probe_ir( + "rarr_value_only", + Type::Array(Box::new(Type::Any)), + body, + None, + ); + assert!( + !ir.contains("rloop.arr."), + "a value-only read must not make its array a region array:\n{ir}" + ); + // Control: the same read consumed by a Number operator does. + let body = vec![call(add(masked(A), Expr::Number(1.0)))]; + let ir = probe_ir("rarr_value_numeric", number_array(), body, None); + assert!( + ir.contains("rloop.arr."), + "a Number-consumed read keeps its array a region array:\n{ir}" + ); + // Control: a value read in a body without a call does too. + let body = vec![ + Stmt::Let { + id: O, + name: "o".to_string(), + ty: Type::Any, + mutable: false, + init: Some(masked(A)), + }, + Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::LocalGet(O)), + property: "d".to_string(), + value: Box::new(Expr::LocalGet(I)), + }), + ]; + let ir = probe_ir( + "rarr_value_no_call", + Type::Array(Box::new(Type::Any)), + body, + None, + ); + assert!( + ir.contains("rloop.arr."), + "a value read in a call-free body keeps its array a region array:\n{ir}" + ); +} diff --git a/crates/perry-codegen/src/stmt/region_loop/arrays.rs b/crates/perry-codegen/src/stmt/region_loop/arrays.rs index 8797a34b8f..f4c5c6fa7e 100644 --- a/crates/perry-codegen/src/stmt/region_loop/arrays.rs +++ b/crates/perry-codegen/src/stmt/region_loop/arrays.rs @@ -380,14 +380,19 @@ pub(super) fn candidates( if cond.is_some_and(|c| !quiet(ctx, c)) || update.is_some_and(|u| !quiet(ctx, u)) { return out; } - // (binding, static max or counter, store) - let mut uses: Vec<(u32, Option, bool)> = Vec::new(); + // (binding, static max or counter, store, a Number operand) + let mut uses: Vec<(u32, Option, bool, bool)> = Vec::new(); let mut walk = |e: &Expr| -> bool { // `numeric`: `e` is an operand a Number consumer reads (arithmetic, // a relational compare, a `Math.*` argument, a stored element). A // counter-indexed read anywhere else (`const o = xs[i]`) is not what // the dense facts serve: it does not make its array a candidate. - fn e_walk(e: &Expr, env: &Env, numeric: bool, out: &mut Vec<(u32, Option, bool)>) { + fn e_walk( + e: &Expr, + env: &Env, + numeric: bool, + out: &mut Vec<(u32, Option, bool, bool)>, + ) { let access = match e { Expr::IndexGet { object, index } => Some((object.as_ref(), index.as_ref(), false)), _ => element_store(e).map(|(o, i, _)| (o, i, true)), @@ -395,7 +400,7 @@ pub(super) fn candidates( if let Some((object, index, store)) = access { if let (Some(Recv::Local(id)), Some(ix)) = (env.array(object), env.index(index)) { if ix.is_some() || store || numeric { - out.push((id, ix, store)); + out.push((id, ix, store, numeric)); } } } @@ -432,9 +437,29 @@ pub(super) fn candidates( r => r, }) .collect(); - for (id, ix, store) in uses { + // What a region serves an array: a store, a read a Number consumer takes, + // or, in a body that runs no JS, any read at a proven index (one load in + // place of the guarded tier, on facts that stay valid across iterations). + // A body that calls out sets the dirty flag and re-checks the guard every + // iteration, which an array only read for its element VALUES + // (`const o = xs[i & 63]; o.m()`) does not pay back: such an array is no + // candidate there, though its reads ride along once some other access + // makes it one. + let calls = body + .iter() + .any(|s| perry_hir::walker::stmt_any_expr(s, &mut may_call)); + let served: HashSet = uses + .iter() + .filter(|&&(_, _, store, numeric)| store || numeric || !calls) + .map(|&(id, ..)| id) + .collect(); + for (id, ix, store, _) in uses { let r = Recv::Local(id); - if written.contains(&id) || keyed.contains(&r) || !receiver_eligible(ctx, r) { + if !served.contains(&id) + || written.contains(&id) + || keyed.contains(&r) + || !receiver_eligible(ctx, r) + { continue; } let u = out.entry(r).or_default(); @@ -447,6 +472,23 @@ pub(super) fn candidates( out } +/// Does `e` contain a call (`f()`, `o.m()`, `new C()`, a native method call)? +fn may_call(e: &Expr) -> bool { + if matches!( + e, + Expr::Call { .. } + | Expr::CallSpread { .. } + | Expr::New { .. } + | Expr::NewDynamic { .. } + | Expr::NativeMethodCall { .. } + ) { + return true; + } + let mut found = false; + perry_hir::walker::walk_expr_children(e, &mut |c| found |= may_call(c)); + found +} + /// The preheader / re-check guard of one array receiver; stores the base. pub(super) fn emit_guard(ctx: &mut FnCtx<'_>, a: &ArrayRecv) -> Result { // The counter: an integer in `[0, i32::MAX]` here (it only grows), and