perf(codegen): #5525 — skip per-element js_number_coerce on untyped typed-array arithmetic operands

crates/perry-codegen/src/expr/binary.rs

@@ -56,6 +56,18 @@ fn lower_arithmetic_operand(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<(String,
             return Ok((value, true));
         }
     }
+    // #5525: an untyped-receiver typed-array element read (`S[i]` with `S` an
+    // `any` param — bcryptjs's Blowfish hot path) used as a non-`+` arithmetic
+    // operand. Lower it as a guaranteed Number (coerce sunk into the cold slow
+    // branch) so the hot per-element fast path skips the site `js_number_coerce`.
+    // Only non-`+` ops reach here (`+` with an untyped operand returned via
+    // `js_dynamic_string_or_number_add` above), and those always `ToNumber`
+    // their operands, so early coercion is semantics-preserving.
+    if let Some(value) =
+        super::index_get::lower_unknown_local_index_get_for_number_context(ctx, expr)?
+    {
+        return Ok((value, true));
+    }
     Ok((lower_expr(ctx, expr)?, false))
 }
 

crates/perry-codegen/src/expr/index_get.rs

@@ -406,7 +406,23 @@ fn lower_guarded_array_index_get(
 /// rejected case defers to the unchanged runtime helper, semantics are
 /// identical; only the hot monomorphic numeric-typed-array case is short-cut.
 /// `obj_box` / `idx_d` are the already-lowered receiver and index (DOUBLE).
-fn lower_inline_dyn_typed_array_get(ctx: &mut FnCtx<'_>, obj_box: &str, idx_d: &str) -> String {
+///
+/// `coerce_slow_to_number`: when the read is used in a context that will
+/// `ToNumber` the result regardless (a non-`+` arithmetic / bitwise operand —
+/// `^`, `-`, `*`, `<<`, …, all of which `ToNumber` their operands; see
+/// [`lower_unknown_local_index_get_for_number_context`]), the cold slow branch's
+/// `js_dyn_index_get` result is wrapped in `js_number_coerce` here so the merged
+/// value is *always* a Number. The hot per-kind fast branches already produce a
+/// Number, so the caller can skip the per-element site `js_number_coerce` it
+/// would otherwise emit — moving that coercion off bcrypt's ~600M-read hot path
+/// and onto the cache-miss path only. `false` leaves the slow result boxed
+/// (the general `obj[i]` read, whose result may legitimately be a non-Number).
+fn lower_inline_dyn_typed_array_get(
+    ctx: &mut FnCtx<'_>,
+    obj_box: &str,
+    idx_d: &str,
+    coerce_slow_to_number: bool,
+) -> String {
     // TAG_MASK / POINTER_TAG / POINTER_MASK as signed-i64 LLVM literals.
     let tag_mask = crate::nanbox::i64_literal(crate::nanbox::TAG_MASK);
     let pointer_tag = crate::nanbox::POINTER_TAG_I64;
@@ -669,11 +685,22 @@ fn lower_inline_dyn_typed_array_get(ctx: &mut FnCtx<'_>, obj_box: &str, idx_d: &
 
     // ---- slow: the unchanged runtime dispatcher ----
     ctx.current_block = slow_idx;
-    let slow_val = ctx.block().call(
+    let slow_raw = ctx.block().call(
         DOUBLE,
         "js_dyn_index_get",
         &[(DOUBLE, obj_box), (DOUBLE, idx_d)],
     );
+    // In a number context, coerce the (possibly boxed) slow result here so the
+    // merge phi is uniformly a Number and the arithmetic caller skips its own
+    // per-element coerce. A plain double already shortcuts `js_number_coerce`'s
+    // first branch, so re-coercing a fast-path-shaped value is a cheap no-op on
+    // the rare cache-miss path.
+    let slow_val = if coerce_slow_to_number {
+        ctx.block()
+            .call(DOUBLE, "js_number_coerce", &[(DOUBLE, &slow_raw)])
+    } else {
+        slow_raw
+    };
     let slow_end_label = ctx.block().label.clone();
     ctx.block().br(&merge_label);
 
@@ -766,6 +793,74 @@ pub(crate) fn lower_numeric_index_get_for_number_context(
     lower_guarded_array_index_get(ctx, &arr_box, &idx_double, &idx_i32, "arr", true, true).map(Some)
 }
 
+/// #5525: lower an `S[i]` read whose receiver is an *untyped* (`any`/unknown)
+/// local — bcryptjs's Blowfish `S`/`P`/`lr` boxes reach their `Int32Array`
+/// state through plain `Array.<number>` parameters — directly as a guaranteed
+/// **Number**, for use as a non-`+` arithmetic / bitwise operand.
+///
+/// The generic `obj[i]` lowering ([`lower_inline_dyn_typed_array_get`] via
+/// [`lower`]) already emits the guarded inline typed-array load, but leaves its
+/// cold slow branch boxed, so the arithmetic site must wrap the whole result in
+/// a per-element `js_number_coerce`. Here we instead sink that coercion into the
+/// slow branch (`coerce_slow_to_number = true`), so the hot per-kind fast path —
+/// ~100% of bcrypt's ~600M reads — pays *no* coerce at all and the caller skips
+/// its site coerce. Operators like `^`/`-`/`*`/`<<` always `ToNumber` their
+/// operands, so coercing early is semantics-preserving; `+` (which may be string
+/// concat) never reaches this path (its untyped operands route through
+/// `js_dynamic_string_or_number_add`).
+///
+/// Returns `None` (caller falls back to `lower_expr` + a site coerce) unless the
+/// receiver is exactly a non-special `any`/unknown `LocalGet` — the one shape
+/// for which [`lower`]'s `IndexGet` arm provably reaches the inline-TA path, so
+/// this never diverges from the value the generic path would have produced.
+pub(crate) fn lower_unknown_local_index_get_for_number_context(
+    ctx: &mut FnCtx<'_>,
+    expr: &Expr,
+) -> Result<Option<String>> {
+    let Expr::IndexGet { object, index } = expr else {
+        return Ok(None);
+    };
+    // Receiver must be a plain local of erased static type. Restricting to
+    // `LocalGet` (not arbitrary expressions) guarantees none of `lower`'s
+    // earlier `IndexGet` branches (Server/globalThis/width-tracked-TA/Uint8Array/
+    // scalar-replaced/flat-const/class-ref/string-receiver) can pre-empt the
+    // inline-TA path, so coercing the slow branch here matches the generic path.
+    let Expr::LocalGet(id) = object.as_ref() else {
+        return Ok(None);
+    };
+    let recv_unknown = matches!(
+        crate::type_analysis::static_type_of(ctx, object),
+        None | Some(HirType::Any) | Some(HirType::Unknown)
+    );
+    if !recv_unknown {
+        return Ok(None);
+    }
+    // Bail if this local is tracked by any specialized lowering that `lower`
+    // would dispatch ahead of the inline-TA path.
+    if ctx.scalar_replaced_arrays.contains_key(id)
+        || ctx.array_row_aliases.contains_key(id)
+        || ctx.scalar_replaced.contains_key(id)
+        || is_string_expr(ctx, object)
+        || index_object_is_class_or_proto_ref(ctx, object)
+    {
+        return Ok(None);
+    }
+    // A statically-string / symbol key is an ordinary [[Get]], not an element
+    // read — leave it to `lower`'s dedicated routes.
+    let index_is_static_string_or_symbol = matches!(
+        index.as_ref(),
+        Expr::String(_) | Expr::WtfString(_) | Expr::SymbolFor(_)
+    ) || is_string_expr(ctx, index);
+    if index_is_static_string_or_symbol {
+        return Ok(None);
+    }
+    let obj_box = lower_expr(ctx, object)?;
+    let idx_d = lower_expr(ctx, index)?;
+    Ok(Some(lower_inline_dyn_typed_array_get(
+        ctx, &obj_box, &idx_d, true,
+    )))
+}
+
 fn lower_bounded_array_index_get(
     ctx: &mut FnCtx<'_>,
     arr_box: &str,
@@ -1201,7 +1296,9 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                 // falling back to `js_dyn_index_get` on any guard miss. Removes
                 // the per-element out-of-line call + `lookup_typed_array_kind` +
                 // `js_number_coerce` on bcrypt's hot Int32Array `S[i]`/`P[i]`.
-                return Ok(lower_inline_dyn_typed_array_get(ctx, &obj_box, &idx_d));
+                return Ok(lower_inline_dyn_typed_array_get(
+                    ctx, &obj_box, &idx_d, false,
+                ));
             }
             // Three cases:
             //   1. Receiver is a known array → inline f64 element load

crates/perry/tests/issue_5525_typed_array_untyped_index.rs

@@ -315,3 +315,114 @@ console.log("D=" + g(own,0) + "," + g(own,1) + "," + g(v,0) + "," + g(v,1));
          view-guard fallback"
     );
 }
+
+/// #5525 follow-up (arithmetic operand coercion): an untyped-receiver typed-
+/// array read (`s[i]` with `s` an `any` param — bcryptjs's Blowfish `S`/`P`
+/// boxes) used as a *non-`+`* arithmetic / bitwise operand previously paid a
+/// per-element `js_number_coerce` at the access site, on top of the inline
+/// typed-array load. The number-context lowering sinks that coercion into the
+/// cold cache-miss slow branch, so the hot per-kind fast path produces a Number
+/// directly and the operator skips its site coerce. This pins that the
+/// optimization is semantics-preserving: the untyped path is bit-identical to
+/// the equivalent typed-receiver path, the slow branch still coerces OOB reads
+/// (`undefined` → `NaN`) and string elements per `ToNumber`, and `+` (which may
+/// be string concat and never takes this path) is unchanged.
+#[test]
+fn untyped_index_in_arithmetic_context_matches_typed() {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let entry = dir.path().join("main.ts");
+    let output = dir.path().join("main_bin");
+
+    std::fs::write(
+        &entry,
+        r#"
+// Identical bodies; `mixAny`'s `any` params take the new number-context path,
+// `mixTyped`'s `Int32Array` params take the existing typed fast path. Every op
+// here (`^`, `-`, `*`, `<<`, `>>>`, `%`) ToNumbers its operands, so the two must
+// agree bit-for-bit — including for the OOB (i=100) reads that resolve through
+// the coerced slow branch (undefined -> NaN).
+function mixAny(s: any, p: any, i: number): number {
+  let x = s[i] ^ p[i];
+  x = x - s[i + 1];
+  x = x * 3;
+  x = x << 2;
+  x = x >>> 1;
+  x = x % 997;
+  return x | 0;
+}
+function mixTyped(s: Int32Array, p: Int32Array, i: number): number {
+  let x = s[i] ^ p[i];
+  x = x - s[i + 1];
+  x = x * 3;
+  x = x << 2;
+  x = x >>> 1;
+  x = x % 997;
+  return x | 0;
+}
+const s = new Int32Array(8);
+const p = new Int32Array(8);
+for (let k = 0; k < 8; k++) { s[k] = (k * 2654435761) | 0; p[k] = ((k + 9) * 40503) | 0; }
+let okA = true;
+for (let i = 0; i < 6; i++) { if (mixAny(s, p, i) !== mixTyped(s, p, i)) okA = false; }
+console.log("A=" + okA);
+
+// (B) in-bounds and out-of-bounds parity in a number context (the OOB case
+// exercises the slow-branch js_number_coerce of an `undefined` element).
+console.log("B=" + (mixAny(s, p, 100) === mixTyped(s, p, 100)));
+
+// (C) a different element kind (Float64) through both paths.
+const f = new Float64Array(4);
+for (let k = 0; k < 4; k++) f[k] = (k + 1) * 1.5;
+function fAny(a: any, i: number): number { return a[i] * a[i + 1] - a[i + 2]; }
+function fTyped(a: Float64Array, i: number): number { return a[i] * a[i + 1] - a[i + 2]; }
+console.log("C=" + (fAny(f, 0) === fTyped(f, 0)));
+
+// (D) a NON-typed-array receiver in a number context: the slow branch must
+// still ToNumber string elements ("5"*2=10, "x"*2=NaN, "10"-3=7).
+const mixed: any = ["5", "x", "10"];
+console.log("D=" + (mixed[0] * 2) + "," + (mixed[1] * 2) + "," + (mixed[2] - 3));
+
+// (E) `+` is NOT a number-context operand path (it may be string concat) and is
+// unaffected: untyped string elements must still concatenate.
+const ss: any = ["ab", "cd"];
+console.log("E=" + (ss[0] + ss[1]));
+"#,
+    )
+    .expect("write entry");
+
+    let compile = Command::new(perry_bin())
+        .current_dir(dir.path())
+        .arg("compile")
+        .arg(&entry)
+        .arg("-o")
+        .arg(&output)
+        .output()
+        .expect("run perry compile");
+    assert!(
+        compile.status.success(),
+        "perry compile failed\nstdout:\n{}\nstderr:\n{}",
+        String::from_utf8_lossy(&compile.stdout),
+        String::from_utf8_lossy(&compile.stderr)
+    );
+
+    let run = Command::new(&output).output().expect("run compiled binary");
+    assert!(
+        run.status.success(),
+        "compiled binary failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}",
+        run.status,
+        String::from_utf8_lossy(&run.stdout),
+        String::from_utf8_lossy(&run.stderr)
+    );
+    let stdout = String::from_utf8_lossy(&run.stdout);
+    assert_eq!(
+        stdout,
+        "A=true\n\
+         B=true\n\
+         C=true\n\
+         D=10,NaN,7\n\
+         E=abcd\n",
+        "untyped typed-array reads in a non-`+` arithmetic context must match \
+         the typed path, coerce OOB/string elements via the slow branch, and \
+         leave `+` semantics unchanged"
+    );
+}