perf(codegen): a loop region whose body calls out takes no array it only reads element values from

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.

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}"
+    );
+}

crates/perry-codegen/src/stmt/region_loop/arrays.rs

@@ -380,22 +380,27 @@ 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<u32>, bool)> = Vec::new();
+    // (binding, static max or counter, store, a Number operand)
+    let mut uses: Vec<(u32, Option<u32>, 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<u32>, bool)>) {
+        fn e_walk(
+            e: &Expr,
+            env: &Env,
+            numeric: bool,
+            out: &mut Vec<(u32, Option<u32>, 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)),
             };
             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<u32> = 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<String> {
     // The counter: an integer in `[0, i32::MAX]` here (it only grows), and