Skip to content

Commit 7f031c1

Browse files
olwangclaude
andcommitted
perf(reg-vm): elision — scalar extraction never taints its source collection (Slice 1)
Generalizes the SH-022 Char-specific fix: a Copy scalar (Int/Bool/Float/ Char/...) has no interior Rc, so extracting it from a collection/struct/ variant cannot alias the source. Thread a scalar_regs bitset from the lowerer into deepcopy_elidable_param_regs and skip the taint edge src->dst for scalar extractions. All read List<Scalar> / Map<_,Scalar> / scalar-field reads now elide their prologue DeepCopy, killing the O(n^2) copy class independent of per-intrinsic classification. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 4411961 commit 7f031c1

4 files changed

Lines changed: 188 additions & 6 deletions

File tree

crates/rsscript/src/reg_vm/lower.rs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,45 @@ pub(crate) struct RegLowerer<'a> {
1212
/// `==`/`!=` could compare a closure-containing operand. Shared across all
1313
/// function lowerings so it summarizes the whole program.
1414
pub(crate) closure_identity_observable: &'a std::cell::Cell<bool>,
15+
/// Registers proven to hold a `Copy` scalar (`Int`/`Bool`/`Float`/`Char`/…,
16+
/// per [`scalar_param_type_needs_no_deep_copy`]). Such a value is inline with
17+
/// no interior `Rc`, so extracting it from a collection/struct/variant cannot
18+
/// alias the source. The DeepCopy-elision taint analysis reads this set to
19+
/// avoid over-tainting an extraction's source collection (see
20+
/// [`deepcopy_elidable_param_regs`]). Conservative by construction: a register
21+
/// is only inserted when the extracted static type is a known scalar; absence
22+
/// means "unknown", which keeps the (sound) over-tainting behavior.
23+
pub(crate) scalar_regs: std::collections::HashSet<Reg>,
24+
}
25+
26+
/// The element type produced by extracting from a collection type, or `None`
27+
/// when it cannot be determined (⇒ conservative: not marked scalar ⇒ copy kept).
28+
/// `List<T>`/`Deque<T>` → `T`; `Map<K, V>`/`SortedMap<K, V>` → `V` (the VALUE, the
29+
/// type `MapGet` yields). Any other root (or a missing generic arg) → `None`.
30+
fn list_elem_type(collection_ty: &str) -> Option<&str> {
31+
let root = crate::text_util::type_root_name(collection_ty);
32+
let args = crate::text_util::type_arg_names(collection_ty)?;
33+
match root {
34+
"List" | "Deque" => args.first().copied(),
35+
"Map" | "SortedMap" => args.get(1).copied(),
36+
_ => None,
37+
}
1538
}
1639

1740
impl RegLowerer<'_> {
41+
/// Record that `dst` holds a `Copy` scalar when `elem_ty` is a known scalar
42+
/// type (`Int`/`Bool`/`Float`/`Char`/…). Extracting such a value is a bit-copy
43+
/// that cannot alias its source, so the elision taint analysis skips it. A
44+
/// `None` / non-scalar `elem_ty` (e.g. `String`, unknown) leaves `dst`
45+
/// unmarked — the sound, over-tainting default.
46+
fn note_scalar(&mut self, dst: Reg, elem_ty: Option<&str>) {
47+
if elem_ty.is_some_and(|ty| {
48+
scalar_param_type_needs_no_deep_copy(crate::text_util::strip_fresh_type(ty))
49+
}) {
50+
self.scalar_regs.insert(dst);
51+
}
52+
}
53+
1854
pub(crate) fn local(&mut self, name: &str) -> Reg {
1955
if let Some(reg) = self.function.local_regs.get(name) {
2056
return *reg;
@@ -502,6 +538,8 @@ impl RegLowerer<'_> {
502538
list,
503539
index,
504540
});
541+
let elem_ty = reg_expr_type_name(iterable).and_then(list_elem_type);
542+
self.note_scalar(item, elem_ty);
505543

506544
self.loop_stack.push(LoopPatch {
507545
cleanup_base: self.cleanup_stack.len(),
@@ -850,13 +888,18 @@ impl RegLowerer<'_> {
850888
name: name.clone(),
851889
});
852890
}
891+
// `access.type_name` is the field's own static type — the value
892+
// this extraction yields — so it feeds `note_scalar` directly.
893+
self.note_scalar(dst, access.type_name.as_deref());
853894
Ok(dst)
854895
}
855896
HirExpr::Index { base, index, .. } => {
856897
let list = self.expr(base)?;
898+
let elem_ty = reg_expr_type_name(base).and_then(list_elem_type);
857899
let index = self.expr(index)?;
858900
let dst = self.temp();
859901
self.emit(RegInstr::ListGet { dst, list, index });
902+
self.note_scalar(dst, elem_ty);
860903
Ok(dst)
861904
}
862905
HirExpr::Effect { value, .. } => self.expr(value),
@@ -927,6 +970,7 @@ impl RegLowerer<'_> {
927970
loop_stack: Vec::new(),
928971
cleanup_stack: Vec::new(),
929972
closure_identity_observable: self.closure_identity_observable,
973+
scalar_regs: std::collections::HashSet::new(),
930974
};
931975
for capture in &capture_names {
932976
lowerer.local(capture);
@@ -1244,6 +1288,11 @@ impl RegLowerer<'_> {
12441288
dst,
12451289
deque: arg_regs[0],
12461290
});
1291+
let elem_ty = args
1292+
.first()
1293+
.and_then(|arg| reg_expr_type_name(&arg.value))
1294+
.and_then(list_elem_type);
1295+
self.note_scalar(dst, elem_ty);
12471296
return Ok(dst);
12481297
}
12491298
("Deque", "pop_front") => {
@@ -1257,6 +1306,11 @@ impl RegLowerer<'_> {
12571306
dst,
12581307
deque: arg_regs[0],
12591308
});
1309+
let elem_ty = args
1310+
.first()
1311+
.and_then(|arg| reg_expr_type_name(&arg.value))
1312+
.and_then(list_elem_type);
1313+
self.note_scalar(dst, elem_ty);
12601314
return Ok(dst);
12611315
}
12621316
("Deque", "push_back") => {
@@ -1381,6 +1435,11 @@ impl RegLowerer<'_> {
13811435
list: arg_regs[0],
13821436
index: arg_regs[1],
13831437
});
1438+
let elem_ty = args
1439+
.first()
1440+
.and_then(|arg| reg_expr_type_name(&arg.value))
1441+
.and_then(list_elem_type);
1442+
self.note_scalar(dst, elem_ty);
13841443
return Ok(dst);
13851444
}
13861445
("List", "len") => {
@@ -1533,6 +1592,14 @@ impl RegLowerer<'_> {
15331592
map: arg_regs[0],
15341593
key: arg_regs[1],
15351594
});
1595+
// `list_elem_type` returns the Map VALUE type (`V` of
1596+
// `Map<K, V>`), which is what `MapGet` yields (as a
1597+
// fresh `Option<V>` of a cloned value).
1598+
let value_ty = args
1599+
.first()
1600+
.and_then(|arg| reg_expr_type_name(&arg.value))
1601+
.and_then(list_elem_type);
1602+
self.note_scalar(dst, value_ty);
15361603
return Ok(dst);
15371604
}
15381605
("Map", "insert") => {

crates/rsscript/src/reg_vm/model.rs

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ pub(crate) fn closure_captures_all_scalar(closure: &VmClosure) -> bool {
398398
})
399399
}
400400

401-
fn scalar_param_type_needs_no_deep_copy(type_name: &str) -> bool {
401+
pub(crate) fn scalar_param_type_needs_no_deep_copy(type_name: &str) -> bool {
402402
matches!(
403403
type_name,
404404
"Bool"
@@ -2481,7 +2481,11 @@ fn deepcopy_instr_forces_keep(instr: &RegInstr, tainted: &[bool], n_regs: usize)
24812481
///
24822482
/// Correctness-critical: when in doubt the analysis keeps the copy. It runs one independent
24832483
/// pass per root so a keep verdict is attributed to exactly the responsible parameter.
2484-
fn deepcopy_elidable_param_regs(code: &[RegInstr], n_regs: usize) -> std::collections::HashSet<Reg> {
2484+
fn deepcopy_elidable_param_regs(
2485+
code: &[RegInstr],
2486+
n_regs: usize,
2487+
scalar_regs: &std::collections::HashSet<Reg>,
2488+
) -> std::collections::HashSet<Reg> {
24852489
let roots: Vec<Reg> = code
24862490
.iter()
24872491
.filter_map(|instr| match instr {
@@ -2500,16 +2504,38 @@ fn deepcopy_elidable_param_regs(code: &[RegInstr], n_regs: usize) -> std::collec
25002504
while changed {
25012505
changed = false;
25022506
for instr in code {
2503-
if let RegInstr::Move { dst, src }
2504-
| RegInstr::ListGet { dst, list: src, .. }
2507+
// A `Move` copies the value whole, so taint always flows (a scalar
2508+
// dst is harmless — a never-tainted scalar already yields the right
2509+
// verdict, but a `Move` of a heap value must propagate).
2510+
if let RegInstr::Move { dst, src } = instr {
2511+
if *src < n_regs && *dst < n_regs && tainted[*src] && !tainted[*dst] {
2512+
tainted[*dst] = true;
2513+
changed = true;
2514+
}
2515+
}
2516+
// Extractions (`ListGet`/`MapGet`/`GetField`/`GetFieldSlot`/
2517+
// `UnwrapVariantValue`/`DequePop*`) pull an INTERIOR value out of a
2518+
// collection/struct/variant. When the lowerer proved `dst` holds a
2519+
// `Copy` scalar (`Int`/`Bool`/`Float`/`Char`/…), that value is inline
2520+
// with no interior `Rc`: it cannot alias `src` or carry `src`'s `Rc`
2521+
// into an escape, so the taint must NOT flow — that spurious edge is
2522+
// exactly what forced the O(n²) copy-keep for per-element helpers.
2523+
// Absence from `scalar_regs` (unknown/`String`/`Bytes`/`Json`/heap)
2524+
// keeps the sound over-tainting behavior.
2525+
else if let RegInstr::ListGet { dst, list: src, .. }
25052526
| RegInstr::MapGet { dst, map: src, .. }
25062527
| RegInstr::GetField { dst, base: src, .. }
25072528
| RegInstr::GetFieldSlot { dst, base: src, .. }
25082529
| RegInstr::UnwrapVariantValue { dst, src, .. }
25092530
| RegInstr::DequePopFront { dst, deque: src }
25102531
| RegInstr::DequePopBack { dst, deque: src } = instr
25112532
{
2512-
if *src < n_regs && *dst < n_regs && tainted[*src] && !tainted[*dst] {
2533+
if *src < n_regs
2534+
&& *dst < n_regs
2535+
&& tainted[*src]
2536+
&& !tainted[*dst]
2537+
&& !scalar_regs.contains(dst)
2538+
{
25132539
tainted[*dst] = true;
25142540
changed = true;
25152541
}
@@ -2608,6 +2634,7 @@ impl RegUnit {
26082634
loop_stack: Vec::new(),
26092635
cleanup_stack: Vec::new(),
26102636
closure_identity_observable: &closure_identity_observable,
2637+
scalar_regs: std::collections::HashSet::new(),
26112638
};
26122639
for param in &signature.params {
26132640
let reg = lowerer.local(&param.name);
@@ -2635,7 +2662,8 @@ impl RegUnit {
26352662
// byte-identical to before.
26362663
if elide_deepcopy_enabled() {
26372664
let n_regs = lowerer.function.regs;
2638-
let elidable = deepcopy_elidable_param_regs(&lowerer.function.code, n_regs);
2665+
let elidable =
2666+
deepcopy_elidable_param_regs(&lowerer.function.code, n_regs, &lowerer.scalar_regs);
26392667
for instr in lowerer.function.code.iter_mut() {
26402668
if let RegInstr::DeepCopy { reg } = instr {
26412669
if elidable.contains(reg) {
@@ -2672,6 +2700,7 @@ impl RegUnit {
26722700
loop_stack: Vec::new(),
26732701
cleanup_stack: Vec::new(),
26742702
closure_identity_observable: &closure_identity_observable,
2703+
scalar_regs: std::collections::HashSet::new(),
26752704
};
26762705
if let Some(info) = hir.type_info(type_name) {
26772706
for field in &info.fields_ordered {

crates/rsscript/src/reg_vm/tests.rs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,68 @@ fn main() -> Unit {
709709
assert_eq!(output.stdout, "98\n");
710710
}
711711

712+
/// Slice 1 generalization (beyond SH-022's Char special-case): a `read
713+
/// List<Int>` param whose extracted elements are RETURNED (a keep-forcing use)
714+
/// must still have its prologue `DeepCopy` ELIDED. Extracting a `Copy` scalar
715+
/// (`Int`) can no longer taint its source collection, so the returned element
716+
/// is untainted and nothing pins the copy — even though `Return` of a tainted
717+
/// register WOULD force a keep. This is the general kill for the O(n^2)
718+
/// `read List<Scalar>` copy class, independent of any per-scalar intrinsic
719+
/// classification. No `Char.*` (or any) intrinsic on the element is involved.
720+
#[test]
721+
fn deepcopy_elision_fires_for_int_list_read_param() {
722+
let source = r#"
723+
features: local
724+
725+
fn scan(xs: read List<Int>, i: Int) -> Int {
726+
let a = List.get<Int>(list: read xs, index: i)
727+
let b = List.get<Int>(list: read xs, index: i + 1)
728+
if a > b {
729+
return a
730+
}
731+
return b
732+
}
733+
734+
fn main() -> Unit {
735+
local xs = List.new<Int>()
736+
List.push<Int>(list: mut xs, value: read 3)
737+
List.push<Int>(list: mut xs, value: read 7)
738+
Log.write(message: read String.from_int(value: scan(xs: read xs, i: 0)))
739+
return Unit
740+
}
741+
"#;
742+
let executable =
743+
reg_vm_compile_source("deepcopy-elision-int.rss", source).expect("lowering succeeds");
744+
745+
let scan_id = executable.unit.function_ids["scan"];
746+
let scan = executable.unit.functions[scan_id].as_ref();
747+
let elided = scan
748+
.code
749+
.iter()
750+
.filter(|instr| matches!(instr, RegInstr::DeepCopyElided { .. }))
751+
.count();
752+
let eager = scan
753+
.code
754+
.iter()
755+
.filter(|instr| matches!(instr, RegInstr::DeepCopy { .. }))
756+
.count();
757+
if crate::reg_vm::model::elide_deepcopy_enabled_for_test() {
758+
assert!(
759+
elided >= 1 && eager == 0,
760+
"elision ON: `read List<Int>` param whose extracted elements are \
761+
returned must be elided (scalar extraction must not taint the list), \
762+
got {elided} elided / {eager} eager: {:#?}",
763+
scan.code,
764+
);
765+
}
766+
767+
// Elision must not change behavior: max(xs[0], xs[1]) == max(3, 7) == 7.
768+
let output = executable
769+
.eval_main_with_args(Vec::<String>::new())
770+
.expect("program should still run");
771+
assert_eq!(output.stdout, "7\n");
772+
}
773+
712774
#[test]
713775
fn jit_runs_scalar_self_recursion_on_flat_executor() {
714776
let source = r#"

docs/ledgers/rss-selfhost-ledger.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,30 @@ gap is VM value-representation / intrinsic-dispatch cost (the next big lever).
658658
(regression guard). Full differential + compiled-parity green (elision soundness).
659659
- **Status:** fixed (O(n²) removed; residual ~46× is the general VM per-op tax,
660660
tracked by the parked perf roadmap).
661+
- **GENERALIZED — Slice 1 of borrow-by-default (2026-07-01):** the SH-022 fix was
662+
intrinsic-specific (it re-classified the 12 `Char.*` intrinsics so a tainted
663+
extracted `Char` stopped pinning the copy). The general root cause was that the
664+
taint pass OVER-tainted: extracting a `Copy` scalar (`Int`/`Bool`/`Float`/`Char`/…)
665+
from a collection/struct/variant (`ListGet`/`MapGet`/`GetField`/`GetFieldSlot`/
666+
`UnwrapVariantValue`/`DequePop*`) tainted the whole source, so ANY keep-forcing
667+
use of the scalar (a `Return`, a store, an unclassified intrinsic) pinned the copy
668+
→ the same O(n²) class for every `read List<Scalar>` / `Map<_, Scalar>` /
669+
scalar-field read, not just Char. **Fix:** a `Copy` scalar has no interior `Rc`,
670+
so extracting one (a bit-copy; for `MapGet`/`DequePop*` a fresh `Option<Scalar>`
671+
of a `.cloned()` scalar) cannot alias the source or carry its `Rc` into an escape.
672+
The lowerer (which has HIR types) now threads a `scalar_regs` bitset — populated at
673+
each extractor site whose extracted static type is a known scalar — into
674+
`deepcopy_elidable_param_regs`, which SKIPS the taint edge `src→dst` for scalar
675+
extractions (`Move` is unchanged; non-scalar values like `String`/`Bytes`/`Json`/
676+
`List<T>` still taint, since their `.cloned()` shares the `Rc`). Now ALL
677+
`read List<Scalar>` / `Map<_, Scalar>` / scalar-field reads elide their prologue
678+
`DeepCopy`, independent of any per-intrinsic classification. Sound: over-tainting
679+
was only a pessimization; the three `does_not_leak` JIT-acceptance guards stay
680+
green. Perf holds at 45.9× (no regression). Sites NOT marked (conservative keep,
681+
sound): list-pattern element extractions, variant-payload unwraps, and
682+
struct-field pattern binds — pattern lowering does not thread the scrutinee's
683+
static type, so those extractions keep the (sound) over-taint. New regression
684+
guard: `reg_vm::tests::…::deepcopy_elision_fires_for_int_list_read_param`.
661685

662686
### SH-023 — self-hosted checker reaches RS0005 parity at declaration level; the merged callable namespace is the load-bearing rule
663687

0 commit comments

Comments
 (0)