Skip to content

Commit a1eaf7e

Browse files
committed
Harden the rc-elision measurement with tests, fix a loop soundness bug
Add tests pinning both directions of the movability classification: a last-use call-argument move, a value used again (not a move), an aliasing move, a field-store escape, and a borrowed element load. Writing the loop test caught a real unsoundness: the linear last-use marked a loop-carried variable's in-loop use as a move, when it is live across the back-edge. Fix it with a loop-aware rule, extracting loop_ranges from loop_depths: a use inside a loop is a move only if the variable is also defined inside that same loop (a per-iteration temporary), never carried in from outside. Conservative by construction, so it under-counts rather than ever over-counting. Record the follow-ups in PROFILING.md: the liveness is a first cut (full dataflow for nested loops, branches, and cross-block moves is the non-trivial next step), more elision categories to find (return moves, orphan-temp direct frees, retain/release cancellation, struct-field moves), the counts being static rather than runtime-weighted, and that the transform reuses the dormant moved flag.
1 parent ee89b3d commit a1eaf7e

2 files changed

Lines changed: 168 additions & 2 deletions

File tree

plans/PROFILING.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,46 @@ is the allocation and reference-count work these counts expose, not raw dispatch
8585
instruction fusion cuts dispatch on the roughly 3,865 instructions. The allocation and RC traffic
8686
is the larger, more concrete target of the two.
8787

88+
## Reference-count elision measurement
89+
90+
`--rc-report` runs a read-only analysis over the typed IR and classifies each emitted retain, so the
91+
elision opportunity is measured before any elision is built. It changes no codegen. Retains fall into
92+
movable categories (an aliasing copy, a call argument, a spawn argument) and kept categories (a
93+
borrowed element load, a store into an aggregate, which are genuine shared references). A movable
94+
retain is one where the source's last real use is the retain site, so passing it is a move rather
95+
than a copy, and it pairs with the moved-from variable's drop, so eliding one elides both.
96+
97+
The move test is loop-aware and conservative, since the whole point is to be safe: a variable carried
98+
into a loop from outside is live across the back-edge, so its last linear use inside the loop is not a
99+
move. The rule requires that for every loop enclosing a use, the variable is also defined inside that
100+
loop, which keeps per-iteration temporaries movable while rejecting loop-carried variables. The
101+
direction is always safe by construction: when the analysis cannot prove a move, it keeps the retain.
102+
Tests pin both directions (a last-use move, a value used again, a loop-carried variable, an aliasing
103+
move, a field-store escape, a borrowed load).
104+
105+
First result on the http hello server: call-arg retains dominate (72 of the 97 movable-category
106+
retains) and 55 are movable, so the calling-convention retain (the caller retains, the callee drops)
107+
is the large lever and most arguments are moves.
108+
109+
### Follow-ups (non-trivial, revisit before building the elision)
110+
111+
- **The liveness is a first cut.** The loop rule is sound but coarse. A full version wants proper
112+
dataflow liveness: nested loops, `break`/`continue`, conditionals and merge points, multiple
113+
reaching definitions, and cross-block moves. The current rule is intentionally conservative, so it
114+
under-counts rather than ever over-counting, but the real elision transform needs the stronger
115+
analysis to catch the cases it currently rejects.
116+
- **Find more elision categories.** Beyond the three movable categories there are more: a value
117+
returned moves ownership out of the function (its drop is elidable), a uniquely-owned orphan
118+
temporary's drop can become a direct free rather than a decrement-and-check, a retain and release
119+
that cancel within a block regardless of category, and a struct construction that moves its fields
120+
in rather than retaining them. Each is its own rule to measure and prove.
121+
- **The counts are static, not runtime-weighted.** They count retain sites in the code, not their
122+
execution frequency, so they show the shape of the opportunity rather than the exact runtime total.
123+
Weighting by the per-request hot path would bridge to the runtime `vm-counters` number.
124+
- **Then build the transform.** The mechanism is already present: the dormant `moved` flag, which the
125+
drop emitter already respects. The elision sets it for proven moves and skips the paired retain,
126+
keyed on the same decision the measurement makes.
127+
88128
## Where this sits
89129

90130
At about 15.8k realistic and 25k raw single-core, an interpreted register VM running a user-space

solid-snake-compiler/src/bytecode_gen.rs

Lines changed: 128 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,10 @@ fn position_in(order: &[usize], v: usize) -> usize {
373373
/// target label sits earlier in the stream, spanning `[label, jump]`. A statement's depth is how
374374
/// many such spans contain it, so nested loops read as deeper. Used to weight accesses when choosing
375375
/// spill victims.
376-
fn loop_depths(ir: &[TypedIRInstruction]) -> Vec<usize> {
376+
/// The instruction ranges of loops, as `(header index, back-jump index)` for each back-edge (a jump
377+
/// to an earlier label). An index inside such a range can re-execute, so a variable read there may be
378+
/// read again on a later iteration, which is what makes a linear last-use unsound as a move signal.
379+
fn loop_ranges(ir: &[TypedIRInstruction]) -> Vec<(usize, usize)> {
377380
let mut label_index: HashMap<String, usize> = HashMap::new();
378381
for (i, instr) in ir.iter().enumerate() {
379382
if let TypedIRStmt::Label(name) = &instr.kind {
@@ -397,7 +400,11 @@ fn loop_depths(ir: &[TypedIRInstruction]) -> Vec<usize> {
397400
}
398401
}
399402
}
403+
back_edges
404+
}
400405

406+
fn loop_depths(ir: &[TypedIRInstruction]) -> Vec<usize> {
407+
let back_edges = loop_ranges(ir);
401408
(0..ir.len())
402409
.map(|idx| {
403410
back_edges
@@ -574,8 +581,35 @@ fn for_each_stmt_read(stmt: &TypedIRStmt, f: &mut impl FnMut(usize)) {
574581
/// in `lower_body`/`lower_expr_to_bytecode_stage_one`.
575582
pub fn analyze_rc_elision(ir: &[TypedIRInstruction]) -> RcElisionReport {
576583
let last = last_real_use(ir);
584+
let loops = loop_ranges(ir);
585+
// Every index at which each variable is defined (written), used to tell a per-iteration
586+
// temporary from a loop-carried variable.
587+
let mut defs: HashMap<usize, Vec<usize>> = HashMap::new();
588+
for (i, instr) in ir.iter().enumerate() {
589+
if let Some(v) = stmt_def_var(&instr.kind) {
590+
defs.entry(v).or_default().push(i);
591+
}
592+
}
577593
let mut r = RcElisionReport::default();
578-
let is_movable = |v: usize, i: usize| last.get(&v) == Some(&i);
594+
// A retain is a move only if the source's last real use is here and, for every loop enclosing
595+
// this use, the source is also defined inside that loop at or before the use. A variable carried
596+
// in from outside a loop is live across the back-edge, so its last linear use is not a move.
597+
let is_movable = |v: usize, i: usize| -> bool {
598+
if last.get(&v) != Some(&i) {
599+
return false;
600+
}
601+
for &(start, end) in &loops {
602+
if start <= i && i <= end {
603+
let defined_in_loop = defs
604+
.get(&v)
605+
.is_some_and(|ds| ds.iter().any(|&d| start <= d && d <= i));
606+
if !defined_in_loop {
607+
return false;
608+
}
609+
}
610+
}
611+
true
612+
};
579613
for (i, instr) in ir.iter().enumerate() {
580614
match &instr.kind {
581615
// Aliasing `q = p` retains the copied reference.
@@ -1791,6 +1825,98 @@ mod codegen_tests {
17911825
analyze_ast_with_opts(&ast, AnalysisOptions::default())
17921826
}
17931827

1828+
/// Compile `src`, then run the reference-count elision analysis on one function's IR. Function
1829+
/// names are mangled by parameter types, so a bare name matches its mangled form. Use "main" for
1830+
/// the top-level body.
1831+
fn rc_report_for(src: &str, fn_name: &str) -> super::RcElisionReport {
1832+
let ctx = analyze_source(src);
1833+
assert!(!ctx.errors().is_fatal(), "compile errors: {:?}", ctx.errors());
1834+
if fn_name == "main" {
1835+
return super::analyze_rc_elision(&ctx.main_function().body);
1836+
}
1837+
let mangled = format!("{fn_name}$");
1838+
let func = ctx
1839+
.functions()
1840+
.iter()
1841+
.find(|f| f.name == fn_name || f.name.starts_with(&mangled))
1842+
.unwrap_or_else(|| panic!("no function named {fn_name}"));
1843+
super::analyze_rc_elision(&func.body)
1844+
}
1845+
1846+
const RC_SINK: &str = "def sink(s: String) -> Int:\n return len(s)\n\n";
1847+
1848+
#[test]
1849+
fn rc_call_arg_last_use_is_movable() {
1850+
// `x`'s only use is the call, so passing it is a move: the retain is elidable.
1851+
let r = rc_report_for(
1852+
&format!("{RC_SINK}def caller() -> Int:\n let x = \"ab\"\n return sink(x)\n"),
1853+
"caller",
1854+
);
1855+
assert_eq!(r.call_arg_total, 1, "one reference argument");
1856+
assert_eq!(r.call_arg_movable, 1, "its last use is the call, so movable");
1857+
}
1858+
1859+
#[test]
1860+
fn rc_value_used_after_is_not_movable() {
1861+
// `x` is passed twice, so the first pass is not a move (it is still live), only the last.
1862+
let r = rc_report_for(
1863+
&format!("{RC_SINK}def twice() -> Int:\n let x = \"ab\"\n let a = sink(x)\n let b = sink(x)\n return a + b\n"),
1864+
"twice",
1865+
);
1866+
assert_eq!(r.call_arg_total, 2, "two reference arguments");
1867+
assert_eq!(r.call_arg_movable, 1, "only the last use is a move");
1868+
}
1869+
1870+
#[test]
1871+
fn rc_loop_carried_is_not_movable() {
1872+
// `x` is read every iteration, so it is live across the back-edge: its last linear use inside
1873+
// the loop is NOT a move. A linear last-use analysis would wrongly mark it movable.
1874+
let r = rc_report_for(
1875+
&format!("{RC_SINK}def loopy() -> Int:\n let x = \"ab\"\n let total = 0\n let i = 0\n while i < 2:\n total = total + sink(x)\n i = i + 1\n return total\n"),
1876+
"loopy",
1877+
);
1878+
assert_eq!(r.call_arg_total, 1, "one reference argument, in the loop");
1879+
assert_eq!(
1880+
r.call_arg_movable, 0,
1881+
"a loop-carried variable is live across iterations, so it must not be counted movable"
1882+
);
1883+
}
1884+
1885+
#[test]
1886+
fn rc_aliasing_last_use_is_movable() {
1887+
// `let b = a` with `a` dead afterward is a move of the reference into `b`. `a` is a parameter
1888+
// so it is not a folded constant (a literal aliasing would fold away and emit no retain).
1889+
let r = rc_report_for(
1890+
"def f(a: String) -> Int:\n let b = a\n return len(b)\n",
1891+
"f",
1892+
);
1893+
assert_eq!(r.alias_total, 1, "one aliasing copy");
1894+
assert_eq!(r.alias_movable, 1, "the source is dead after, so it is a move");
1895+
}
1896+
1897+
#[test]
1898+
fn rc_field_store_is_kept_not_movable() {
1899+
// Storing a reference into a struct field makes the struct share it, an escape that stays
1900+
// counted rather than becoming a move.
1901+
let r = rc_report_for(
1902+
"type Box:\n v: String\n\ndef setit(b: Box, s: String):\n b.v = s\n",
1903+
"setit",
1904+
);
1905+
assert_eq!(r.aggregate_store_kept, 1, "the field store is a kept escape");
1906+
assert_eq!(r.call_arg_movable, 0, "no movable call argument here");
1907+
}
1908+
1909+
#[test]
1910+
fn rc_indexed_load_is_kept() {
1911+
// A loaded element is a borrowed reference into a live collection, a genuine new reference
1912+
// that stays counted.
1913+
let r = rc_report_for(
1914+
"def geti(arr: Array[String]) -> Int:\n let x = arr[0]\n return len(x)\n",
1915+
"geti",
1916+
);
1917+
assert_eq!(r.indexed_load_kept, 1, "the element load is a kept borrow");
1918+
}
1919+
17941920
#[test]
17951921
fn fallible_parse_int_returns_result() {
17961922
// The fallible-builtin convention: __parse_int sets a status that the prelude parse_int

0 commit comments

Comments
 (0)