Skip to content

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

@proggeramlug

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_loop
  • every_store_into_a_hoisted_scalar_slot_shades_its_value
  • repeated_stores_into_one_scalar_slot_bind_once
  • numeric_only_scalar_replaced_literal_emits_no_entry_rooting
  • numeric_only_scalar_replaced_object_emits_no_rooting
  • numeric_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_alloca call 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_slots gets its two causes separated: state how many slots the flat-const table itself reserves and how many the temp pool adds.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugConfirmed defect or regression

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions