Skip to content

Commit 05602bd

Browse files
author
Ralph Küpper
committed
perf(codegen): version stable packed array loops
Lands #8719 with its soundness blocker fixed. The blocker was NOT where the emitted IR first suggested. The symptom was an unguarded `fptosi` + `xor i32` consuming a phi that merged a proven number (the string arm of a guarded `charCodeAt`) with an arbitrary user method's result (the generic arm) -- so an `any` receiver returning a BigInt yielded garbage where the spec requires BigInt xor or a TypeError. The cause is one lever in `expr/literals_vars.rs`. The PR's new `explicit_numeric_toint32` disjunct bypassed `can_lower_expr_as_i32_in_current_region`, which correctly answers false for `(h ^ recv.charCodeAt(i)) | 0`, and called into the `lower_expr_as_i32` family without its documented precondition. `lower_expr_native_i32`'s i32-chain arm then recursed structurally, `fptosi`-ing every operand it could not lower natively -- so `binary.rs`, which already applies `is_provably_not_bigint` per operand, was bypassed entirely rather than mishandling the join. `expr_produces_canonical_raw_f64` is a claim about the VALUE the ordinary lowering produces, not a licence to re-evaluate its operands natively. The fix lowers through `lower_expr` and applies `toint32_fast`, so an unproven tree keeps `js_dynamic_bitxor` while a proven one still gets the inline `xor i32`. The i32-slot store the lever exists for is preserved, so this remains an improvement over main, which took neither branch here. House precedent settles the alternative: `lower_guarded_numeric_add`'s doc block records that sinking arithmetic into the arms was already tried and is worse -- per-node diamonds make the outer add consume a phi that LLVM cannot prove canonical, and that shape went 86 ms -> 119 ms. The negative control passes UNMODIFIED (blob hash identical to main), and both positive tests still fire the fast path for proven receivers. No version bump.
1 parent e2eee40 commit 05602bd

48 files changed

Lines changed: 3111 additions & 126 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
### Performance
2+
3+
- Loop-version stable counted iteration over packed `Array` and `Array` subclasses, with fallback-free direct reads and mutation-safe current-index side exits.
4+
5+
### Fixed
6+
7+
- The `x | 0` canonical-ToInt32 store lever no longer re-evaluates its operand
8+
tree as native i32. `expr_produces_canonical_raw_f64` vouches for the RESULT
9+
of `x | 0`, not for `x`'s operands, so consuming it as a licence to call
10+
`lower_expr_native(_, I32)` violated that path's documented precondition
11+
(`can_lower_expr_as_i32_in_current_region`). The i32-chain arm `fptosi`s
12+
every operand it cannot lower natively, which turned
13+
`h ^ recv.charCodeAt(i)` on an `any` receiver into an inline `xor i32` over
14+
whatever that method returned — a BigInt silently produced garbage instead
15+
of the spec's `TypeError`. The lever now lowers through `lower_expr`, which
16+
applies `is_provably_not_bigint` per operand, and takes one trailing
17+
`toint32_fast` to feed the i32 slot; `x | 0` always lowers to `sitofp i32`,
18+
so that pair folds away. A proven tree still gets the inline `xor i32` and
19+
keeps the i32-slot store. Pinned by the pre-existing negative control
20+
`char_code_at_on_an_unproven_receiver_keeps_the_runtime_lowering`.

crates/perry-codegen/src/codegen/artifacts.rs

Lines changed: 33 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use super::entry::compile_module_entry;
1919
use super::helpers::{
2020
function_body_returns_generator_object, sanitize, scoped_fn_name, unknown_func_wrapper_name,
2121
};
22+
use super::indexed_method_artifacts::{compile_indexed_method_clones, IndexedMethodArtifactsCtx};
2223
use super::method::{
2324
compile_method, compile_static_method, compile_typed_f64_method,
2425
compile_typed_f64_receiver_method, compile_typed_i1_method, compile_typed_i32_method,
@@ -298,38 +299,29 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
298299
.nonnegative_index_methods
299300
.get(&(class.name.clone(), method.name.clone()))
300301
{
301-
compile_method(
302-
llmod,
303-
class,
304-
method,
305-
func_names,
306-
strings,
307-
class_table,
308-
method_names,
309-
module_globals,
310-
module_global_types,
311-
opts.import_function_prefixes,
312-
enum_table,
313-
static_field_globals,
314-
class_ids,
315-
func_signatures,
316-
func_synthetic_arguments,
317-
module_boxed_vars,
318-
closure_rest_params,
319-
cross_module,
320-
None,
321-
false,
322-
None,
323-
Some(nonnegative_index_params),
324-
false,
325-
false,
326-
)
327-
.with_context(|| {
328-
format!(
329-
"lowering nonnegative-index method clone '{}::{}'",
330-
class.name, method.name
331-
)
332-
})?;
302+
compile_indexed_method_clones(
303+
IndexedMethodArtifactsCtx {
304+
llmod,
305+
class,
306+
method,
307+
func_names,
308+
strings,
309+
classes: class_table,
310+
methods: method_names,
311+
module_globals,
312+
module_global_types,
313+
import_function_prefixes: opts.import_function_prefixes,
314+
enums: enum_table,
315+
static_field_globals,
316+
class_ids,
317+
func_signatures,
318+
func_synthetic_arguments,
319+
module_boxed_vars,
320+
closure_rest_params,
321+
cross_module,
322+
},
323+
nonnegative_index_params,
324+
)?;
333325
}
334326
compile_method(
335327
llmod,
@@ -358,6 +350,7 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
358350
None,
359351
false,
360352
false,
353+
false,
361354
)
362355
.with_context(|| format!("lowering method '{}::{}'", class.name, method.name))?;
363356
if cross_module
@@ -388,6 +381,7 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
388381
None,
389382
None,
390383
false,
384+
false,
391385
true,
392386
)
393387
.with_context(|| {
@@ -433,6 +427,7 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
433427
None,
434428
false,
435429
false,
430+
false,
436431
)
437432
.with_context(|| {
438433
format!(
@@ -468,6 +463,7 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
468463
Some(fact.clone()),
469464
None,
470465
false,
466+
false,
471467
true,
472468
)
473469
.with_context(|| {
@@ -510,6 +506,7 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
510506
false,
511507
Some(fact.clone()),
512508
None,
509+
false,
513510
true,
514511
false,
515512
)
@@ -552,6 +549,7 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
552549
None,
553550
false,
554551
false,
552+
false,
555553
)
556554
.with_context(|| {
557555
format!(
@@ -620,6 +618,7 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
620618
None,
621619
false,
622620
false,
621+
false,
623622
)
624623
.with_context(|| format!("lowering getter '{}::{}'", class.name, prop))?;
625624
}
@@ -676,6 +675,7 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
676675
None,
677676
false,
678677
false,
678+
false,
679679
)
680680
.with_context(|| format!("lowering setter '{}::{}'", class.name, prop))?;
681681
}
@@ -774,6 +774,7 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> {
774774
None,
775775
false,
776776
false,
777+
false,
777778
)
778779
.with_context(|| format!("lowering constructor for '{}'", class.name))?;
779780
}

crates/perry-codegen/src/codegen/closure.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,6 +1081,7 @@ pub(super) fn compile_closure(
10811081
class_shape_slots: HashMap::new(),
10821082
class_header_images: HashMap::new(),
10831083
cached_lengths: HashMap::new(),
1084+
array_length_snapshots: HashMap::new(),
10841085
bounded_index_pairs: Vec::new(),
10851086
packed_f64_loop_facts: Vec::new(),
10861087
masked_window_array_facts: Vec::new(),
@@ -1154,6 +1155,9 @@ pub(super) fn compile_closure(
11541155
typed_f64_methods: &cross_module.typed_f64_methods,
11551156
pshape_methods: &cross_module.pshape_methods,
11561157
nonnegative_index_methods: &cross_module.nonnegative_index_methods,
1158+
trusted_array_param_handles: HashMap::new(),
1159+
versioned_indexed_loop_facts: Vec::new(),
1160+
stable_packed_loop_facts: Vec::new(),
11571161
pshape_tower_routable: &cross_module.pshape_tower_routable,
11581162
proven_this: None,
11591163
typed_i32_methods: &cross_module.typed_i32_methods,

crates/perry-codegen/src/codegen/entry.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -872,6 +872,7 @@ pub(super) fn compile_module_entry(
872872
class_shape_slots: HashMap::new(),
873873
class_header_images: HashMap::new(),
874874
cached_lengths: HashMap::new(),
875+
array_length_snapshots: HashMap::new(),
875876
bounded_index_pairs: Vec::new(),
876877
packed_f64_loop_facts: Vec::new(),
877878
masked_window_array_facts: Vec::new(),
@@ -954,6 +955,9 @@ pub(super) fn compile_module_entry(
954955
typed_f64_methods: &cross_module.typed_f64_methods,
955956
pshape_methods: &cross_module.pshape_methods,
956957
nonnegative_index_methods: &cross_module.nonnegative_index_methods,
958+
trusted_array_param_handles: HashMap::new(),
959+
versioned_indexed_loop_facts: Vec::new(),
960+
stable_packed_loop_facts: Vec::new(),
957961
pshape_tower_routable: &cross_module.pshape_tower_routable,
958962
proven_this: None,
959963
typed_i32_methods: &cross_module.typed_i32_methods,
@@ -1571,6 +1575,7 @@ pub(super) fn compile_module_entry(
15711575
class_shape_slots: HashMap::new(),
15721576
class_header_images: HashMap::new(),
15731577
cached_lengths: HashMap::new(),
1578+
array_length_snapshots: HashMap::new(),
15741579
bounded_index_pairs: Vec::new(),
15751580
packed_f64_loop_facts: Vec::new(),
15761581
masked_window_array_facts: Vec::new(),
@@ -1653,6 +1658,9 @@ pub(super) fn compile_module_entry(
16531658
typed_f64_methods: &cross_module.typed_f64_methods,
16541659
pshape_methods: &cross_module.pshape_methods,
16551660
nonnegative_index_methods: &cross_module.nonnegative_index_methods,
1661+
trusted_array_param_handles: HashMap::new(),
1662+
versioned_indexed_loop_facts: Vec::new(),
1663+
stable_packed_loop_facts: Vec::new(),
16561664
pshape_tower_routable: &cross_module.pshape_tower_routable,
16571665
proven_this: None,
16581666
typed_i32_methods: &cross_module.typed_i32_methods,

crates/perry-codegen/src/codegen/function.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1126,6 +1126,7 @@ pub(super) fn compile_function(
11261126
class_shape_slots: HashMap::new(),
11271127
class_header_images: HashMap::new(),
11281128
cached_lengths: HashMap::new(),
1129+
array_length_snapshots: HashMap::new(),
11291130
bounded_index_pairs: Vec::new(),
11301131
packed_f64_loop_facts: Vec::new(),
11311132
masked_window_array_facts: Vec::new(),
@@ -1204,6 +1205,9 @@ pub(super) fn compile_function(
12041205
typed_f64_methods: &cross_module.typed_f64_methods,
12051206
pshape_methods: &cross_module.pshape_methods,
12061207
nonnegative_index_methods: &cross_module.nonnegative_index_methods,
1208+
trusted_array_param_handles: HashMap::new(),
1209+
versioned_indexed_loop_facts: Vec::new(),
1210+
stable_packed_loop_facts: Vec::new(),
12071211
pshape_tower_routable: &cross_module.pshape_tower_routable,
12081212
proven_this: None,
12091213
typed_i32_methods: &cross_module.typed_i32_methods,

crates/perry-codegen/src/codegen/helpers.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -776,6 +776,31 @@ pub(super) fn scoped_static_method_name(
776776
)
777777
}
778778

779+
pub(super) fn node_stream_parent_kind(
780+
classes: &HashMap<String, &perry_hir::Class>,
781+
class: &perry_hir::Class,
782+
) -> Option<&'static str> {
783+
let mut cur = class.extends_name.as_deref();
784+
let mut depth = 0usize;
785+
while let Some(name) = cur {
786+
match name {
787+
"Readable" => return Some("readable"),
788+
"Duplex" => return Some("duplex"),
789+
"Transform" => return Some("transform"),
790+
_ => {}
791+
}
792+
cur = classes
793+
.get(name)
794+
.copied()
795+
.and_then(|parent| parent.extends_name.as_deref());
796+
depth += 1;
797+
if depth > 32 {
798+
break;
799+
}
800+
}
801+
None
802+
}
803+
779804
/// Walk a function body looking for `Return(Some(expr))` shapes that
780805
/// identify the function as a factory returning a class. Sets
781806
/// `*produced` to the resolved class name when the first qualifying

0 commit comments

Comments
 (0)