Repository navigation
scalar_replaced_slot_roots counts js_shadow_slot_bind module-wide, and #7487's pooled temp roots emit it too — 7 tests measure the wrong slots #7504
Description
Split out of #7493. Six failures in crates/perry-codegen/tests/scalar_replaced_slot_roots.rs and one in crates/perry-codegen/tests/shadow_slot_hygiene.rs that survive the lowering pin — they fail identically under NativeRootsPin::shadow() and under PERRY_RS4GC=0, so they are not #7370's.
Symptom
scalar_replaced_slot_roots (all shadow-pinned after #7493):
bind_is_hoisted_into_the_entry_block_ahead_of_the_storing_loopevery_store_into_a_hoisted_scalar_slot_shades_its_valuerepeated_stores_into_one_scalar_slot_bind_oncenumeric_only_scalar_replaced_literal_emits_no_entry_rootingnumeric_only_scalar_replaced_object_emits_no_rootingnumeric_only_scalar_replaced_array_emits_no_rooting
shadow_slot_hygiene:
flat_const_row_aliases_do_not_reserve_shadow_slots
The last two numeric_only_* were passing before #7493 — vacuously. They assert bind_calls(&ir) == 0, and bind_calls counts call void @js_shadow_slot_bind(, which the native-roots default emits for nothing. Pinning them to the lowering they measure is what made them fail.
Cause
bind_calls and the js_shadow_frame_enter(i32 N) slot count are whole-module counters, and #7487 gave temporaries a claim on the same two things. A pooled temp root reserves a frame slot via reserve_shadow_slot() and emits emit_shadow_slot_bind_ptr at every store — the identical call a scalar-replaced field's slot emits.
Worked example. numeric_only_scalar_replaced_object_emits_no_rooting builds const p = {x: 1, y: 2}; console.log(p.x, p.y) and demands zero binds. The emitted module contains three:
%r11 = alloca i64 ; the console arg-array accumulator
%r3 = call ptr @js_shadow_frame_enter(i32 2)
%r10 = call i64 @js_array_alloc(i32 2)
call void @js_shadow_slot_bind(i32 1, ptr %r11)
%r35 = call i64 @js_array_push_f64(i64 %r34, double %r33)
call void @js_shadow_slot_bind(i32 1, ptr %r11)
%r59 = call i64 @js_array_push_f64(i64 %r58, double %r57)
call void @js_shadow_slot_bind(i32 1, ptr %r11)
call void @js_console_log_spread(i64 %r81)All three name %r11, the console.log argument accumulator. Not one of them has anything to do with the scalar-replaced literal. #6997's property — a numeric-only literal pays no rooting — still holds; the counter can no longer see it.
flat_const_row_aliases_do_not_reserve_shadow_slots is the same class one level up: it asserts the frame is entered with exactly one slot (js_shadow_frame_enter(i32 1)), and reserve_shadow_slot's on-demand growth for temp roots rewrites that count in place. Its in-file comment already records it as pre-existing red with three slots instead of one, diagnosed before #7487 landed; whatever the original two extra slots were, the temp pool now adds to them, so the two causes need separating before either can be fixed.
What to build
Make the measurement name its subject instead of counting the module.
The file already has the right idiom in scalar_replaced_split_parts_are_bound, which compares against a control (bind_calls(&ir) > bind_calls(&control)). Generalising it is probably the cheapest correct fix: for each fixture, compile a twin with the scalar-replaced local's heap field replaced by a number, and assert on the difference. Temp-root binds are identical in both, so they cancel.
The stronger fix is to attribute each bind to the alloca it names and select only allocas that belong to the scalar-replacement slots — the test can find those from the store that initialises them. That also fixes flat_const_row_aliases, which needs slot reservations attributed the same way.
Acceptance
- All seven green, with the assertion measuring the scalar-replaced / flat-const slots specifically.
- Sabotage-checked: reverting gc: scalar-replaced object/array locals holding heap values are not precise roots #6968's
root_entry_allocacall must fail the positive tests, and re-adding rooting to a numeric-only literal must fail the negative ones. The negative direction is the one that was silently broken, so it is the one that most needs the sabotage proof. flat_const_row_aliases_do_not_reserve_shadow_slotsgets its two causes separated: state how many slots the flat-const table itself reserves and how many the temp pool adds.
Activity
docs(test): point the lowering notes at the filed follow-ups (#7502, #…
fix(test): make the root-lowering pin reachable from integration suit…
proggeramlug commentedon Aug 9, 2026
Fixed in #7675. Verdict: wrong subject — the counter was measuring the
console.logargument accumulator, not the scalar-replaced slots.What replaced it. New
perry_codegen::testing::root_slots(insrc/, so both the integration suites and in-crate unit tests use one copy) keys everyjs_shadow_slot_bindand everyjs_write_barrier_root_nanboxby the entry alloca it names, and classifies that alloca:Value(alloca double— a named local or a scalar-replaced field/element) orTempRoot(alloca i64, null-seeded at entry — #7487's pool).bound_slotspanics on a bound alloca it cannot classify rather than defaulting it into either bucket, so when a third slot family appears the tests go red naming it instead of silently folding it into whichever total is asserted. That is the property that keeps this from decaying the way the counters did.All seven tests are green, measuring their own slots. The three
numeric_only_*tests additionally asserttemp_root_slot_binds(main) > 0— their zero is only interesting in a module where binds exist, which is precisely what was missing before #7493 pinned them.Sabotage A (positive direction), run. Stubbing out
root_scalar_replaced_slot'sroot_entry_allocacall — #6968 reverted — fails 7 tests:scalar_replaced_object_field_holding_a_heap_value_is_bound,scalar_replaced_array_element_holding_a_heap_value_is_bound,later_store_into_a_scalar_replaced_field_is_bound,repeated_stores_into_one_scalar_slot_bind_once,every_store_into_a_hoisted_scalar_slot_shades_its_value,bind_is_hoisted_into_the_entry_block_ahead_of_the_storing_loop,scalar_replaced_array_element_slots_are_initialized_before_the_bind. (scalar_replaced_split_parts_are_boundcorrectly stays green — it goes through the unconditional arm, which the sabotage did not touch.)Sabotage B (negative direction), run. Disabling the
expr_is_known_non_pointer_shadow_valuegate — #6997 reverted, so a numeric-only literal gets rooted — fails all threenumeric_only_*. That was the silently-broken direction, so it is the one that most needed the proof.flat_const_row_aliases_do_not_reserve_shadow_slots— the two causes, separated.The temp pool is not one of them. Measured in
@mainfor that fixture:js_shadow_frame_enter(i32 3), binds on slot 1 (%r7) and slot 2 (%r8), a clear on slot 0 — andtemp_root_slot_binds == 0. The pooled temp roots contribute 0 binds and 0 reservations here, because the fixture has no allocating call between operands. All three reserved slots are locals'; the issue's hypothesis that "the temp pool now adds to them" does not hold for this fixture.That leaves the original cause on its own, and it turns out the assertion was not the contract:
kernellowers to a real heap array (js_inline_arena_slow_alloc), sokrow = kernel[0]holds a heap pointer andk = krow[1]is anAnycodegen cannot prove numeric. Leaving either unrooted is #6968 exactly. The assertion presumed a flat-const lowering that emits the rows as static data; this fixture does not receive one. That gap is real and worth its own issue, but it is an optimization gap and it was making a rooting-hygiene suite red for a performance reason.A third finding while separating them: the test's second assertion had gone toothless independently of any of this. It forbade
js_shadow_slot_set(i32 1, while #7013 moved the per-slot traffic tojs_shadow_slot_bind— so the row aliases were touching slot 1 the whole time, through a spelling the negative did not name.The test is renamed
flat_const_locals_reserve_and_use_every_slot_they_claimand asserts what this suite exists for: the frame is non-empty, the temp pool's contribution is stated and is zero, and every reserved slot is bound or cleared — a reserved-but-untouched slot being the #7184 shape, where the collector scans a frame entry no store ever reached.test(codegen): make the stale codegen-test contracts able to fail aga…
proggeramlug commentedon Aug 9, 2026
Merged as 6cdcd79 (#7675). Evidence and the two sabotage runs are in the comment above; the
flat_constfinding — the temp pool contributes zero there, and the property it asserted would be a #6968 bug if codegen satisfied it — is recorded there too.