From ce8f92da4d13097d2c3af4c120e9065a13d360fc Mon Sep 17 00:00:00 2001 From: Daniel Morris Date: Wed, 13 May 2026 21:04:10 +0100 Subject: [PATCH 1/3] fix: fall back to per-field record emission to lift VM register-overflow cap Expr::Record and Expr::With emitted OP_RECNEW / OP_RECWITH reading field values from consecutive registers a+1..a+n. That required next_reg + n <= 255, so any record literal with more than ~127 fields, or a moderate one preceded by many locals, hit a hard assert! panic at compile time. Same root-cause shape as PR #237 for list literals. Fix follows the OP_LISTNEW/OP_LISTAPPEND split: three new opcodes (OP_RECNEW_EMPTY = 158, OP_RECCOPY = 159, OP_RECSETFIELD = 160) let the compiler fall back to a per-field setter loop when the contiguous-register layout would overflow. The fallback only needs two registers (result + scratch) regardless of field count. Fast path is preserved: small records still emit a single OP_RECNEW or OP_RECWITH. Only oversized literals or with-updates hit the fallback. With-fallback is gated on all_resolved; the unresolved field-name path keeps the existing dispatch. RC safety on OP_RECSETFIELD: only emitted against records just allocated by OP_RECNEW_EMPTY or OP_RECCOPY, so in-place mutation has no aliasing concern. drop_rc on the old slot + clone_rc on the new value mirrors the OP_RECWITH bookkeeping, just split across two bytecodes. For OP_RECNEW_EMPTY, the old slot is always Nil, so drop_rc is a no-op. --- src/vm/mod.rs | 546 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 455 insertions(+), 91 deletions(-) diff --git a/src/vm/mod.rs b/src/vm/mod.rs index ef3f8cac..8950ae55 100644 --- a/src/vm/mod.rs +++ b/src/vm/mod.rs @@ -305,6 +305,21 @@ pub(crate) const OP_CALL_BUILTIN_TREE: u8 = 155; pub(crate) const OP_RECFLD_SAFE: u8 = 156; pub(crate) const OP_RECFLD_NAME_SAFE: u8 = 157; +// Fallback emission for large records / `with` updates whose value count +// exceeds the consecutive-register layout used by OP_RECNEW / OP_RECWITH. +// These three opcodes cooperate to build a record incrementally using only +// two registers (result + scratch) regardless of field count, mirroring the +// OP_LISTNEW/OP_LISTAPPEND split that PR #237 introduced for list literals. +// +// SAFETY note on OP_RECSETFIELD: it mutates a record field in place without +// any refcount dance. That's only sound because the compiler emits it +// exclusively against records that were just allocated by OP_RECNEW_EMPTY +// or OP_RECCOPY, with no opportunity for sharing in between. Never expose +// this opcode through a user-facing construct without RC-aware checks. +pub(crate) const OP_RECNEW_EMPTY: u8 = 161; // ABx: R[A] = new record of type Bx, all fields = Nil +pub(crate) const OP_RECCOPY: u8 = 162; // ABC: R[A] = clone of record R[B] (fresh allocation, fields clone_rc'd) +pub(crate) const OP_RECSETFIELD: u8 = 163; // ABC: R[A].field[C] = R[B] (in-place, freshly-allocated A only) + /// Builtins eligible for tree-bridge dispatch when no specialised VM opcode /// matches the call shape. The list is deliberately narrow: it covers the /// regex/format/file-format family that has no FnRef args, so NanVal↔Value @@ -2897,42 +2912,78 @@ impl RegCompiler { self.type_registry.types[type_id as usize].fields.clone(); let source_fields: HashMap<&str, &Expr> = fields.iter().map(|(n, e)| (n.as_str(), e)).collect(); - let ordered_regs: Vec = canonical_order - .iter() - .map(|fname| { - let expr = source_fields[fname.as_str()]; - self.compile_expr(expr) - }) - .collect(); - - let a = self.alloc_reg(); // result register - let fields_base = self.next_reg; - assert!( - (self.next_reg as usize) + ordered_regs.len() <= 255, - "register overflow: record literal requires too many register slots" - ); - self.next_reg += ordered_regs.len() as u8; - if self.next_reg > self.max_reg { - self.max_reg = self.next_reg; - } - for (i, &field_reg) in ordered_regs.iter().enumerate() { - let target = fields_base + i as u8; - if field_reg != target { - self.emit_abc(OP_MOVE, target, field_reg, 0); - } - } + let n = canonical_order.len(); + let pre_reg = self.next_reg as usize; + // Fast path budget mirrors Expr::List (see comment there): each + // field compile allocates ~1 register, then we need n more + // contiguous slots for the RECNEW layout. n_fields and type_id + // each also have to fit in 8 bits inside the Bx field. + let fits_contiguous = n <= 255 && type_id <= 255 && pre_reg + 2 * n < 255; assert!( type_id <= 255, "type_id {} exceeds 8-bit limit in OP_RECNEW", type_id ); - let bx = (type_id << 8) | ordered_regs.len() as u16; - self.emit_abx(OP_RECNEW, a, bx); - // Track the type of this register - self.reg_record_type[a as usize] = type_id; - a + + if fits_contiguous { + let ordered_regs: Vec = canonical_order + .iter() + .map(|fname| { + let expr = source_fields[fname.as_str()]; + self.compile_expr(expr) + }) + .collect(); + + let a = self.alloc_reg(); // result register + let fields_base = self.next_reg; + assert!( + (self.next_reg as usize) + ordered_regs.len() <= 255, + "register overflow: record literal requires too many register slots" + ); + self.next_reg += ordered_regs.len() as u8; + if self.next_reg > self.max_reg { + self.max_reg = self.next_reg; + } + + for (i, &field_reg) in ordered_regs.iter().enumerate() { + let target = fields_base + i as u8; + if field_reg != target { + self.emit_abc(OP_MOVE, target, field_reg, 0); + } + } + + let bx = (type_id << 8) | ordered_regs.len() as u16; + self.emit_abx(OP_RECNEW, a, bx); + self.reg_record_type[a as usize] = type_id; + a + } else { + // Fallback for large records that would blow the 255-reg + // frame ceiling on the contiguous layout: + // OP_RECNEW_EMPTY a, type_id ; all fields = Nil + // for each canonical field i: + // scratch = compile_expr(value) + // OP_RECSETFIELD a, scratch, i ; in-place mutate + // reset next_reg so each item reuses the scratch slot + // + // RC-safety: OP_RECSETFIELD mutates in place without + // dropping the old slot value. That's safe here because + // OP_RECNEW_EMPTY initialised every slot to Nil (drop_rc + // is a no-op on Nil) and we never visit the same slot + // twice in the loop below. + let a = self.alloc_reg(); + self.emit_abx(OP_RECNEW_EMPTY, a, type_id); + let after_result = self.next_reg; + for (i, fname) in canonical_order.iter().enumerate() { + let expr = source_fields[fname.as_str()]; + let val_reg = self.compile_expr(expr); + self.emit_abc(OP_RECSETFIELD, a, val_reg, i as u8); + self.next_reg = after_result; + } + self.reg_record_type[a as usize] = type_id; + a + } } Expr::Match { subject, arms } => { @@ -2991,11 +3042,6 @@ impl RegCompiler { let obj_reg = self.compile_expr(object); let obj_type = self.reg_record_type[obj_reg as usize]; - let update_regs: Vec = updates - .iter() - .map(|(_, val_expr)| self.compile_expr(val_expr)) - .collect(); - // Resolve update field names to indices let update_indices: Vec> = updates .iter() @@ -3010,59 +3056,106 @@ impl RegCompiler { .collect(); let all_resolved = update_indices.iter().all(|i| i.is_some()); - // Store as constant: indices (numbers) for resolved, names (strings) for unresolved - let const_val = if all_resolved { - Value::List( - update_indices - .iter() - .map(|i| Value::Number(i.unwrap() as f64)) - .collect(), - ) - } else { - // Fallback: store field names for runtime resolution - Value::List( - updates - .iter() - .map(|(n, _)| Value::Text(n.clone())) - .collect(), - ) - }; - let const_idx = self.current.add_const_raw(const_val); + // Decide between the fast (consecutive-register) layout and + // the RECCOPY + per-field RECSETFIELD fallback. The fast path + // is preferred whenever it fits because OP_RECWITH does the + // copy + overwrite in a single bytecode dispatch. The + // fallback is only triggered for oversized with-expressions + // and currently requires all_resolved — unresolved field + // names in a >127-update with would be an extreme edge case + // we can address with a name-keyed setfield later if needed. + let n = updates.len(); + let pre_reg = self.next_reg as usize; + let fits_contiguous = pre_reg + 2 * n < 255; + let use_fallback = !fits_contiguous && all_resolved; - let a = self.alloc_reg(); // result register - let updates_base = self.next_reg; - assert!( - (self.next_reg as usize) + updates.len() <= 255, - "register overflow: 'with' expression requires too many register slots" - ); - self.next_reg += updates.len() as u8; - if self.next_reg > self.max_reg { - self.max_reg = self.next_reg; - } + if !use_fallback { + let update_regs: Vec = updates + .iter() + .map(|(_, val_expr)| self.compile_expr(val_expr)) + .collect(); - // Move object into result slot - if obj_reg != a { - self.emit_abc(OP_MOVE, a, obj_reg, 0); - } + // Store as constant: indices (numbers) for resolved, names (strings) for unresolved + let const_val = if all_resolved { + Value::List( + update_indices + .iter() + .map(|i| Value::Number(i.unwrap() as f64)) + .collect(), + ) + } else { + // Fallback: store field names for runtime resolution + Value::List( + updates + .iter() + .map(|(n, _)| Value::Text(n.clone())) + .collect(), + ) + }; + let const_idx = self.current.add_const_raw(const_val); - // Move update values into consecutive slots - for (i, &val_reg) in update_regs.iter().enumerate() { - let target = updates_base + i as u8; - if val_reg != target { - self.emit_abc(OP_MOVE, target, val_reg, 0); + let a = self.alloc_reg(); // result register + let updates_base = self.next_reg; + assert!( + (self.next_reg as usize) + updates.len() <= 255, + "register overflow: 'with' expression requires too many register slots" + ); + self.next_reg += updates.len() as u8; + if self.next_reg > self.max_reg { + self.max_reg = self.next_reg; } - } - assert!( - const_idx <= 255, - "constant pool overflow: field data index {} exceeds 8-bit limit in OP_RECWITH", - const_idx - ); - let bx = (const_idx << 8) | updates.len() as u16; - self.emit_abx(OP_RECWITH, a, bx); - // Propagate type (with doesn't change the type) - self.reg_record_type[a as usize] = obj_type; - a + // Move object into result slot + if obj_reg != a { + self.emit_abc(OP_MOVE, a, obj_reg, 0); + } + + // Move update values into consecutive slots + for (i, &val_reg) in update_regs.iter().enumerate() { + let target = updates_base + i as u8; + if val_reg != target { + self.emit_abc(OP_MOVE, target, val_reg, 0); + } + } + + assert!( + const_idx <= 255, + "constant pool overflow: field data index {} exceeds 8-bit limit in OP_RECWITH", + const_idx + ); + let bx = (const_idx << 8) | updates.len() as u16; + self.emit_abx(OP_RECWITH, a, bx); + // Propagate type (with doesn't change the type) + self.reg_record_type[a as usize] = obj_type; + a + } else { + // Fallback emission for large `with` updates: + // OP_RECCOPY a, obj ; new record, fields cloned from obj + // for each update i with field index idx: + // scratch = compile_expr(value) + // OP_RECSETFIELD a, scratch, idx + // reset next_reg so each update reuses the scratch slot + // + // RC-safety: OP_RECCOPY clones every field of `obj` + // (clone_rc on heap values), producing a fresh record + // with refcount 1. OP_RECSETFIELD then in-place + // overwrites individual slots, dropping the cloned + // value via drop_rc and storing the new one — same + // bookkeeping as the existing OP_RECWITH dispatch, just + // split across two bytecodes. The result aliases nothing + // because the copy is fresh. + let a = self.alloc_reg(); + self.emit_abc(OP_RECCOPY, a, obj_reg, 0); + let after_result = self.next_reg; + for (i, (_, val_expr)) in updates.iter().enumerate() { + let idx = update_indices[i].unwrap(); + let val_reg = self.compile_expr(val_expr); + self.emit_abc(OP_RECSETFIELD, a, val_reg, idx); + self.next_reg = after_result; + } + self.reg_record_type[a as usize] = obj_type; + a + } } } } @@ -3078,15 +3171,16 @@ fn chunk_is_all_numeric(chunk: &Chunk) -> bool { for &inst in &chunk.code { let op = (inst >> 24) as u8; match op { - OP_RECNEW | OP_LISTNEW | OP_RECWITH | OP_WRAPOK | OP_WRAPERR | OP_STR | OP_CAT - | OP_SPL | OP_REV | OP_SRT | OP_SRTDESC | OP_SLC | OP_TAKE | OP_DROP | OP_UNQ - | OP_UNIQBY | OP_FRQ | OP_PARTITION | OP_LISTAPPEND | OP_JPAR | OP_JDMP | OP_CSVDMP - | OP_ENV | OP_GET | OP_GETH | OP_GETMANY | OP_POST | OP_POSTH | OP_RD | OP_RDL - | OP_RDJL | OP_WR | OP_WRL | OP_MAPNEW | OP_MGET | OP_MSET | OP_MKEYS | OP_MVALS - | OP_HD | OP_AT | OP_LST | OP_TL | OP_FMT2 | OP_RGXSUB | OP_ZIP | OP_ENUMERATE - | OP_WINDOW | OP_FFT | OP_IFFT | OP_RANGE | OP_CHUNKS | OP_CUMSUM | OP_SETUNION - | OP_SETINTER | OP_SETDIFF | OP_TRANSPOSE | OP_MATMUL | OP_INV | OP_SOLVE - | OP_DTFMT | OP_DTPARSE | OP_CALL_BUILTIN_TREE => { + OP_RECNEW | OP_LISTNEW | OP_RECWITH | OP_RECNEW_EMPTY | OP_RECCOPY | OP_RECSETFIELD + | OP_WRAPOK | OP_WRAPERR | OP_STR | OP_CAT | OP_SPL | OP_REV | OP_SRT | OP_SRTDESC + | OP_SLC | OP_TAKE | OP_DROP | OP_UNQ | OP_UNIQBY | OP_FRQ | OP_PARTITION + | OP_LISTAPPEND | OP_JPAR | OP_JDMP | OP_CSVDMP | OP_ENV | OP_GET | OP_GETH + | OP_GETMANY | OP_POST | OP_POSTH | OP_RD | OP_RDL | OP_RDJL | OP_WR | OP_WRL + | OP_MAPNEW | OP_MGET | OP_MSET | OP_MKEYS | OP_MVALS | OP_HD | OP_AT | OP_LST + | OP_TL | OP_FMT2 | OP_RGXSUB | OP_ZIP | OP_ENUMERATE | OP_WINDOW | OP_FFT + | OP_IFFT | OP_RANGE | OP_CHUNKS | OP_CUMSUM | OP_SETUNION | OP_SETINTER + | OP_SETDIFF | OP_TRANSPOSE | OP_MATMUL | OP_INV | OP_SOLVE | OP_DTFMT | OP_DTPARSE + | OP_CALL_BUILTIN_TREE => { return false; } _ => {} @@ -5721,6 +5815,149 @@ impl<'a> VM<'a> { reg_set!(a, new_record); } // end else (heap record path) } + OP_RECNEW_EMPTY => { + // R[A] = new record of type Bx with all fields = Nil. + // Companion to OP_RECNEW for the oversized-literal fallback. + let a = ((inst >> 16) & 0xFF) as usize + base; + let type_id = (inst & 0xFFFF) as u16; + let n_fields = self.program.type_registry.types[type_id as usize] + .fields + .len(); + + if let Some(rec_ptr) = self.arena.alloc_record(type_id, n_fields) { + unsafe { + let rec = &mut *rec_ptr; + for i in 0..n_fields { + *rec.field_ptr_mut(i) = NanVal::nil().0; + } + } + reg_set!(a, NanVal::arena_record(rec_ptr)); + } else { + let type_info = + Rc::clone(&self.program.type_registry.types[type_id as usize]); + let fields: Vec = (0..n_fields).map(|_| NanVal::nil()).collect(); + reg_set!(a, NanVal::heap_record(type_info, fields.into_boxed_slice())); + } + } + OP_RECCOPY => { + // R[A] = fresh record cloned from R[B]. All fields clone_rc'd + // so the new record can be mutated by OP_RECSETFIELD without + // disturbing the source. Companion to OP_RECWITH for the + // oversized-update fallback. + let a = ((inst >> 16) & 0xFF) as usize + base; + let b = ((inst >> 8) & 0xFF) as usize + base; + let src = reg!(b); + + if src.is_arena_record() { + let (type_id, n_fields) = unsafe { + let rec = src.as_arena_record(); + (rec.type_id, rec.n_fields as usize) + }; + if let Some(new_ptr) = self.arena.alloc_record(type_id, n_fields) { + unsafe { + let old_rec = src.as_arena_record(); + let new_rec = &mut *new_ptr; + for i in 0..n_fields { + let v = NanVal(*old_rec.field_ptr(i)); + v.clone_rc(); + *new_rec.field_ptr_mut(i) = v.0; + } + } + reg_set!(a, NanVal::arena_record(new_ptr)); + } else { + // Arena full — promote to heap + let type_info = + Rc::clone(&self.program.type_registry.types[type_id as usize]); + let mut new_fields = Vec::with_capacity(n_fields); + unsafe { + let old_rec = src.as_arena_record(); + for i in 0..n_fields { + let v = NanVal(*old_rec.field_ptr(i)); + v.clone_rc(); + new_fields.push(v); + } + } + reg_set!( + a, + NanVal::heap_record(type_info, new_fields.into_boxed_slice()) + ); + } + } else { + debug_assert!(src.is_heap(), "OP_RECCOPY on non-record value"); + let new_record = unsafe { + match src.as_heap_ref() { + HeapObj::Record { type_info, fields } => { + let new_fields: Vec = fields.to_vec(); + for v in new_fields.iter() { + v.clone_rc(); + } + NanVal::heap_record( + Rc::clone(type_info), + new_fields.into_boxed_slice(), + ) + } + _ => vm_err!(VmError::Type("'with' requires a record")), + } + }; + reg_set!(a, new_record); + } + } + OP_RECSETFIELD => { + // R[A].field[C] = R[B], in place. + // The compiler only emits this against freshly-allocated + // records (rc=1, no aliasing), so we skip defensive RC + // checks. We still drop_rc the old slot value to keep + // refcounts balanced — for Nil (post-RECNEW_EMPTY) that's + // a no-op; for cloned heap values from RECCOPY it + // releases the just-cloned reference before the new + // value (clone_rc'd here) takes over. + let a = ((inst >> 16) & 0xFF) as usize + base; + let b = ((inst >> 8) & 0xFF) as usize + base; + let c = (inst & 0xFF) as usize; + let val = reg!(b); + let rec = reg!(a); + + if rec.is_arena_record() { + // SAFETY: arena records live in the bump arena for + // the full duration of a VM run and are never + // moved. Casting the const pointer to mut is sound + // because the compiler only emits OP_RECSETFIELD + // against records it just allocated via + // OP_RECNEW_EMPTY/OP_RECCOPY — no aliasing. + unsafe { + let rec_ptr = (rec.0 & PTR_MASK) as *mut ArenaRecord; + let r = &mut *rec_ptr; + debug_assert!( + c < r.n_fields as usize, + "OP_RECSETFIELD field index out of range" + ); + NanVal(*r.field_ptr(c)).drop_rc(); + val.clone_rc(); + *r.field_ptr_mut(c) = val.0; + } + } else { + debug_assert!(rec.is_heap(), "OP_RECSETFIELD on non-record value"); + unsafe { + match rec.as_heap_ref() { + HeapObj::Record { fields, .. } => { + debug_assert!( + c < fields.len(), + "OP_RECSETFIELD field index out of range" + ); + // SAFETY: We just allocated this record via + // OP_RECNEW_EMPTY/OP_RECCOPY and the compiler + // guarantees no other reference exists yet. + // Cast the &[NanVal] to *mut NanVal to mutate. + let slot_ptr = fields.as_ptr().add(c) as *mut NanVal; + (*slot_ptr).drop_rc(); + val.clone_rc(); + *slot_ptr = val; + } + _ => vm_err!(VmError::Type("OP_RECSETFIELD on non-record")), + } + } + } + } OP_LISTNEW => { let a = ((inst >> 16) & 0xFF) as usize + base; let n = (inst & 0xFFFF) as usize; @@ -10846,6 +11083,133 @@ pub(crate) extern "C" fn jit_recwith_arena( jit_recwith(rec, indices_ptr, n_updates, regs) } +/// Allocate a new record of type `type_id` with all fields = Nil. +/// `arena_ptr` is a pointer to a BumpArena, `registry_ptr` is a pointer +/// to &TypeRegistry. Companion to `jit_recnew` for the oversized-literal +/// fallback emitted by the compiler when the consecutive-register layout +/// would blow the 255-reg frame ceiling. +#[cfg(feature = "cranelift")] +#[unsafe(no_mangle)] +pub(crate) extern "C" fn jit_recnew_empty(arena_ptr: u64, type_id: u64, registry_ptr: u64) -> u64 { + let tid = type_id as u16; + let registry = unsafe { &*(registry_ptr as *const TypeRegistry) }; + let n = registry.types[tid as usize].fields.len(); + let arena = unsafe { &mut *(arena_ptr as *mut BumpArena) }; + + if let Some(rec_ptr) = arena.alloc_record(tid, n) { + unsafe { + let rec = &mut *rec_ptr; + for i in 0..n { + *rec.field_ptr_mut(i) = TAG_NIL; + } + } + return NanVal::arena_record(rec_ptr).0; + } + + // Arena full — heap fallback + let type_info = Rc::clone(®istry.types[tid as usize]); + let fields: Vec = (0..n).map(|_| NanVal::nil()).collect(); + NanVal::heap_record(type_info, fields.into_boxed_slice()).0 +} + +/// Clone a record (fresh allocation, all fields clone_rc'd). Companion to +/// `jit_recwith` for the oversized-update fallback: callers follow this +/// with a series of `jit_recsetfield` calls to overwrite individual slots. +#[cfg(feature = "cranelift")] +#[unsafe(no_mangle)] +pub(crate) extern "C" fn jit_reccopy(src: u64, arena_ptr: u64, registry_ptr: u64) -> u64 { + let rv = NanVal(src); + let arena = unsafe { &mut *(arena_ptr as *mut BumpArena) }; + let registry = unsafe { &*(registry_ptr as *const TypeRegistry) }; + + if rv.is_arena_record() { + let (tid, n) = unsafe { + let rec = rv.as_arena_record(); + (rec.type_id, rec.n_fields as usize) + }; + if let Some(new_ptr) = arena.alloc_record(tid, n) { + unsafe { + let old_rec = rv.as_arena_record(); + let new_rec = &mut *new_ptr; + for i in 0..n { + let v = NanVal(*old_rec.field_ptr(i)); + v.clone_rc(); + *new_rec.field_ptr_mut(i) = v.0; + } + } + return NanVal::arena_record(new_ptr).0; + } + // Arena full — promote + let type_info = Rc::clone(®istry.types[tid as usize]); + let mut new_fields = Vec::with_capacity(n); + unsafe { + let old_rec = rv.as_arena_record(); + for i in 0..n { + let v = NanVal(*old_rec.field_ptr(i)); + v.clone_rc(); + new_fields.push(v); + } + } + return NanVal::heap_record(type_info, new_fields.into_boxed_slice()).0; + } + + if !rv.is_heap() { + return TAG_NIL; + } + + match unsafe { rv.as_heap_ref() } { + HeapObj::Record { type_info, fields } => { + let new_fields: Vec = fields.to_vec(); + for v in new_fields.iter() { + v.clone_rc(); + } + NanVal::heap_record(Rc::clone(type_info), new_fields.into_boxed_slice()).0 + } + _ => TAG_NIL, + } +} + +/// In-place mutate field `idx` of record `rec` to `val`. SAFETY: only valid +/// against records that were just allocated by `jit_recnew_empty` or +/// `jit_reccopy` (rc=1, no aliasing). Drops the old slot value's refcount +/// and clones the new one. +#[cfg(feature = "cranelift")] +#[unsafe(no_mangle)] +pub(crate) extern "C" fn jit_recsetfield(rec: u64, val: u64, idx: u64) { + let rv = NanVal(rec); + let vv = NanVal(val); + let i = idx as usize; + + if rv.is_arena_record() { + unsafe { + // SAFETY: arena records live for the duration of a VM run and + // are pinned. The compiler-emitted RECSETFIELD only targets + // freshly-allocated records, so the mut cast does not alias. + let rec_ptr = (rv.0 & PTR_MASK) as *mut ArenaRecord; + let r = &mut *rec_ptr; + debug_assert!(i < r.n_fields as usize, "jit_recsetfield idx oob"); + NanVal(*r.field_ptr(i)).drop_rc(); + vv.clone_rc(); + *r.field_ptr_mut(i) = vv.0; + } + return; + } + + if !rv.is_heap() { + return; + } + + unsafe { + if let HeapObj::Record { fields, .. } = rv.as_heap_ref() { + debug_assert!(i < fields.len(), "jit_recsetfield idx oob"); + let slot_ptr = fields.as_ptr().add(i) as *mut NanVal; + (*slot_ptr).drop_rc(); + vv.clone_rc(); + *slot_ptr = vv; + } + } +} + /// Create a new list from n items pointed to by `regs`. #[cfg(feature = "cranelift")] #[unsafe(no_mangle)] From f2754507cbf3a797af2ea115d640b7bc241ff958 Mon Sep 17 00:00:00 2001 From: Daniel Morris Date: Wed, 13 May 2026 21:04:16 +0100 Subject: [PATCH 2/3] cranelift: dispatch arms for the new record-fallback opcodes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JIT and AOT both need to know about OP_RECNEW_EMPTY, OP_RECCOPY, and OP_RECSETFIELD or they'd bail to interpreter on any program with an oversized record literal. The fallback path is cold (only fires on records with >127 fields or under heavy register pressure), so a simple C-ABI helper call suits both backends — no need to mirror the inline-arena bump allocation OP_RECNEW gets on the fast path. jit_recnew_empty, jit_reccopy, and jit_recsetfield mirror the VM dispatch one-for-one. OP_RECSETFIELD returns no result and skips def_var on its A reg: the record's NanVal is unchanged by in-place field mutation, so the prior RECNEW_EMPTY/RECCOPY definition stays current. --- src/vm/compile_cranelift.rs | 68 +++++++++++++++++++++++++++++++++---- src/vm/jit_cranelift.rs | 61 ++++++++++++++++++++++++++++++++- 2 files changed, 121 insertions(+), 8 deletions(-) diff --git a/src/vm/compile_cranelift.rs b/src/vm/compile_cranelift.rs index e4028c16..f90b538b 100644 --- a/src/vm/compile_cranelift.rs +++ b/src/vm/compile_cranelift.rs @@ -108,6 +108,9 @@ struct HelperFuncs { recfld: FuncId, recfld_name: FuncId, recnew: FuncId, + recnew_empty: FuncId, + reccopy: FuncId, + recsetfield: FuncId, recwith: FuncId, listnew: FuncId, listget: FuncId, @@ -279,6 +282,9 @@ fn declare_all_helpers(module: &mut ObjectModule) -> HelperFuncs { recfld: declare_helper(module, "jit_recfld", 2, 1), recfld_name: declare_helper(module, "jit_recfld_name", 3, 1), recnew: declare_helper(module, "jit_recnew", 4, 1), + recnew_empty: declare_helper(module, "jit_recnew_empty", 3, 1), + reccopy: declare_helper(module, "jit_reccopy", 3, 1), + recsetfield: declare_helper(module, "jit_recsetfield", 3, 0), recwith: declare_helper(module, "jit_recwith", 4, 1), listnew: declare_helper(module, "jit_listnew", 2, 1), listget: declare_helper(module, "jit_listget", 2, 1), @@ -1016,13 +1022,13 @@ fn compile_function_body( | OP_SRT | OP_SRTDESC | OP_SLC | OP_TAKE | OP_DROP | OP_SPL | OP_CAT | OP_GET | OP_POST | OP_GETH | OP_POSTH | OP_GETMANY | OP_ENV | OP_JPTH | OP_JDMP | OP_JPAR | OP_RDJL | OP_MAPNEW | OP_MGET | OP_MSET | OP_MDEL | OP_MKEYS - | OP_MVALS | OP_LISTNEW | OP_LISTAPPEND | OP_RECNEW | OP_RECWITH | OP_PRT - | OP_RD | OP_RDL | OP_WR | OP_WRL | OP_TRM | OP_UPR | OP_LWR | OP_CAP | OP_PADL - | OP_PADR | OP_CHR | OP_CHARS | OP_UNQ | OP_UNIQBY | OP_PARTITION | OP_FRQ - | OP_NUM | OP_RGXSUB | OP_ZIP | OP_ENUMERATE | OP_RANGE | OP_WINDOW | OP_CHUNKS - | OP_CUMSUM | OP_SETUNION | OP_SETINTER | OP_SETDIFF | OP_FFT | OP_IFFT - | OP_TRANSPOSE | OP_MATMUL | OP_INV | OP_SOLVE | OP_DTFMT | OP_DTPARSE - | OP_CALL_BUILTIN_TREE => { + | OP_MVALS | OP_LISTNEW | OP_LISTAPPEND | OP_RECNEW | OP_RECWITH + | OP_RECNEW_EMPTY | OP_RECCOPY | OP_PRT | OP_RD | OP_RDL | OP_WR | OP_WRL + | OP_TRM | OP_UPR | OP_LWR | OP_CAP | OP_PADL | OP_PADR | OP_CHR | OP_CHARS + | OP_UNQ | OP_UNIQBY | OP_PARTITION | OP_FRQ | OP_NUM | OP_RGXSUB | OP_ZIP + | OP_ENUMERATE | OP_RANGE | OP_WINDOW | OP_CHUNKS | OP_CUMSUM | OP_SETUNION + | OP_SETINTER | OP_SETDIFF | OP_FFT | OP_IFFT | OP_TRANSPOSE | OP_MATMUL + | OP_INV | OP_SOLVE | OP_DTFMT | OP_DTPARSE | OP_CALL_BUILTIN_TREE => { non_num_write[a] = true; non_bool_write[a] = true; } @@ -2620,6 +2626,54 @@ fn compile_function_body( let result = builder.inst_results(call_inst)[0]; builder.def_var(vars[a_idx], result); } + // ── Record creation (oversized-literal fallback) ── + OP_RECNEW_EMPTY => { + // Companion to OP_RECNEW for the fallback path the compiler + // emits when the consecutive-register layout would exceed + // the 255-reg frame ceiling. Cold path → simple helper call. + let type_id = (inst & 0xFFFF) as i64; + let fref_arena = get_func_ref(&mut builder, module, helpers.get_arena_ptr); + let arena_call = builder.ins().call(fref_arena, &[]); + let arena_ptr_val = builder.inst_results(arena_call)[0]; + let fref_reg = get_func_ref(&mut builder, module, helpers.get_registry_ptr); + let reg_call = builder.ins().call(fref_reg, &[]); + let registry_ptr_val = builder.inst_results(reg_call)[0]; + let type_id_val = builder.ins().iconst(I64, type_id); + let fref = get_func_ref(&mut builder, module, helpers.recnew_empty); + let call_inst = builder + .ins() + .call(fref, &[arena_ptr_val, type_id_val, registry_ptr_val]); + let result = builder.inst_results(call_inst)[0]; + builder.def_var(vars[a_idx], result); + } + // ── Record copy (oversized-with fallback) ── + OP_RECCOPY => { + let b_idx = ((inst >> 8) & 0xFF) as usize; + let src = builder.use_var(vars[b_idx]); + let fref_arena = get_func_ref(&mut builder, module, helpers.get_arena_ptr); + let arena_call = builder.ins().call(fref_arena, &[]); + let arena_ptr_val = builder.inst_results(arena_call)[0]; + let fref_reg = get_func_ref(&mut builder, module, helpers.get_registry_ptr); + let reg_call = builder.ins().call(fref_reg, &[]); + let registry_ptr_val = builder.inst_results(reg_call)[0]; + let fref = get_func_ref(&mut builder, module, helpers.reccopy); + let call_inst = builder + .ins() + .call(fref, &[src, arena_ptr_val, registry_ptr_val]); + let result = builder.inst_results(call_inst)[0]; + builder.def_var(vars[a_idx], result); + } + // ── Record set-field (in-place mutate, freshly-allocated only) ── + OP_RECSETFIELD => { + let b_idx = ((inst >> 8) & 0xFF) as usize; + let c = (inst & 0xFF) as i64; + let rec_val = builder.use_var(vars[a_idx]); + let val_val = builder.use_var(vars[b_idx]); + let idx_val = builder.ins().iconst(I64, c); + let fref = get_func_ref(&mut builder, module, helpers.recsetfield); + builder.ins().call(fref, &[rec_val, val_val, idx_val]); + // No result — R[A] still points to the same (now-mutated) record. + } // ── List creation ── OP_LISTNEW => { let n = (inst & 0xFFFF) as usize; diff --git a/src/vm/jit_cranelift.rs b/src/vm/jit_cranelift.rs index 4b4cd4cd..187c6e90 100644 --- a/src/vm/jit_cranelift.rs +++ b/src/vm/jit_cranelift.rs @@ -115,6 +115,9 @@ struct HelperFuncs { recfld: FuncId, recfld_name: FuncId, recnew: FuncId, + recnew_empty: FuncId, + reccopy: FuncId, + recsetfield: FuncId, recwith: FuncId, recwith_arena: FuncId, listnew: FuncId, @@ -279,6 +282,9 @@ fn register_helpers(builder: &mut JITBuilder) { ("jit_recfld", jit_recfld as *const u8), ("jit_recfld_name", jit_recfld_name as *const u8), ("jit_recnew", jit_recnew as *const u8), + ("jit_recnew_empty", jit_recnew_empty as *const u8), + ("jit_reccopy", jit_reccopy as *const u8), + ("jit_recsetfield", jit_recsetfield as *const u8), ("jit_recwith", jit_recwith as *const u8), ("jit_recwith_arena", jit_recwith_arena as *const u8), ("jit_listnew", jit_listnew as *const u8), @@ -428,6 +434,9 @@ fn declare_all_helpers(module: &mut JITModule) -> HelperFuncs { recfld: declare_helper(module, "jit_recfld", 2, 1), recfld_name: declare_helper(module, "jit_recfld_name", 3, 1), recnew: declare_helper(module, "jit_recnew", 4, 1), + recnew_empty: declare_helper(module, "jit_recnew_empty", 3, 1), + reccopy: declare_helper(module, "jit_reccopy", 3, 1), + recsetfield: declare_helper(module, "jit_recsetfield", 3, 0), recwith: declare_helper(module, "jit_recwith", 4, 1), recwith_arena: declare_helper(module, "jit_recwith_arena", 5, 1), listnew: declare_helper(module, "jit_listnew", 2, 1), @@ -1052,7 +1061,7 @@ fn compile_function_body( | OP_ENV | OP_JPTH | OP_JDMP | OP_JPAR | OP_RDJL | OP_MAPNEW | OP_MGET | OP_MSET | OP_MDEL | OP_MKEYS | OP_MVALS | OP_LISTNEW | OP_LISTAPPEND - | OP_RECNEW | OP_RECWITH + | OP_RECNEW | OP_RECWITH | OP_RECNEW_EMPTY | OP_RECCOPY | OP_PRT | OP_RD | OP_RDL | OP_WR | OP_WRL | OP_TRM | OP_UPR | OP_LWR | OP_CAP | OP_PADL | OP_PADR | OP_CHR | OP_CHARS | OP_UNQ | OP_UNIQBY | OP_PARTITION | OP_FRQ | OP_NUM | OP_RGXSUB | OP_TRANSPOSE | OP_MATMUL | OP_DTFMT | OP_DTPARSE @@ -3079,6 +3088,56 @@ fn compile_function_body( builder.def_var(vars[a_idx], result); } } + OP_RECNEW_EMPTY => { + // Fallback emission for oversized record literals. The fast + // path (OP_RECNEW above) inlines the arena bump; this path + // is cold (only fires on records with >127 fields or lots + // of preceding locals) so a simple helper call is fine. + let type_id = (inst & 0xFFFF) as i64; + let arena_ptr_val = builder.ins().iconst(I64, jit_arena_ptr() as i64); + let type_id_val = builder.ins().iconst(I64, type_id); + let registry_ptr_val = builder + .ins() + .iconst(I64, &program.type_registry as *const TypeRegistry as i64); + let fref = get_func_ref(&mut builder, module, helpers.recnew_empty); + let call_inst = builder + .ins() + .call(fref, &[arena_ptr_val, type_id_val, registry_ptr_val]); + let result = builder.inst_results(call_inst)[0]; + builder.def_var(vars[a_idx], result); + } + OP_RECCOPY => { + // R[A] = fresh clone of R[B]. Companion to OP_RECNEW_EMPTY + // for the oversized-with fallback. Same cold-path rationale + // as RECNEW_EMPTY: helper call rather than inlined. + let b_idx = ((inst >> 8) & 0xFF) as usize; + let src = builder.use_var(vars[b_idx]); + let arena_ptr_val = builder.ins().iconst(I64, jit_arena_ptr() as i64); + let registry_ptr_val = builder + .ins() + .iconst(I64, &program.type_registry as *const TypeRegistry as i64); + let fref = get_func_ref(&mut builder, module, helpers.reccopy); + let call_inst = builder + .ins() + .call(fref, &[src, arena_ptr_val, registry_ptr_val]); + let result = builder.inst_results(call_inst)[0]; + builder.def_var(vars[a_idx], result); + } + OP_RECSETFIELD => { + // R[A].field[C] = R[B], in place. Only emitted against + // freshly-allocated records (rc=1, no aliasing), so the + // helper does no defensive RC dance — see the SAFETY + // comment on OP_RECSETFIELD in src/vm/mod.rs. + let b_idx = ((inst >> 8) & 0xFF) as usize; + let c = (inst & 0xFF) as i64; + let rec_val = builder.use_var(vars[a_idx]); + let val_val = builder.use_var(vars[b_idx]); + let idx_val = builder.ins().iconst(I64, c); + let fref = get_func_ref(&mut builder, module, helpers.recsetfield); + builder.ins().call(fref, &[rec_val, val_val, idx_val]); + // No result — the record at R[A] is mutated in place; the + // var still holds the same NanVal so we don't redefine it. + } OP_LISTNEW => { let n = (inst & 0xFFFF) as usize; if n == 0 { From a7137fb358d64d5bdad37f2d010e245339ed9857 Mon Sep 17 00:00:00 2001 From: Daniel Morris Date: Wed, 13 May 2026 21:04:24 +0100 Subject: [PATCH 3/3] test: cross-engine coverage for large records and oversized with 18 regression tests pin the fix across tree, VM, and Cranelift: size sweep at 1/16/60/150/220 fields for both record literals and with-updates, plus the 180-leading-locals shape that originally surfaced the bug in agent-written code. Adjacent coverage: - With on a 150-field record preserves untouched fields (exercises OP_RECCOPY's clone_rc bookkeeping). - With does not mutate the original record (the fallback allocates fresh storage; OP_RECSETFIELD only touches the copy). - Record with mixed string/number fields exercises clone_rc across the fallback boundary for heap field values. Two examples (large-record-literal.ilo, large-record-with.ilo) get picked up by tests/examples_engines.rs so the run/out assertions fire on every engine on every CI run. --- examples/large-record-literal.ilo | 21 ++ examples/large-record-with.ilo | 18 + tests/regression_record_register_overflow.rs | 362 +++++++++++++++++++ 3 files changed, 401 insertions(+) create mode 100644 examples/large-record-literal.ilo create mode 100644 examples/large-record-with.ilo create mode 100644 tests/regression_record_register_overflow.rs diff --git a/examples/large-record-literal.ilo b/examples/large-record-literal.ilo new file mode 100644 index 00000000..b8d7025e --- /dev/null +++ b/examples/large-record-literal.ilo @@ -0,0 +1,21 @@ +-- Large record literals work at any size. Once a record literal would +-- exceed the VM 255-register-per-frame budget for the consecutive-slot +-- layout, the compiler falls back to OP_RECNEW_EMPTY + per-field +-- OP_RECSETFIELD emission. From the persona view, big-record literals +-- with hundreds of fields just work, no chunking via `with` or +-- builder helpers required. + +type big{f0:n;f1:n;f2:n;f3:n;f4:n;f5:n;f6:n;f7:n;f8:n;f9:n;f10:n;f11:n;f12:n;f13:n;f14:n;f15:n;f16:n;f17:n;f18:n;f19:n;f20:n;f21:n;f22:n;f23:n;f24:n;f25:n;f26:n;f27:n;f28:n;f29:n;f30:n;f31:n;f32:n;f33:n;f34:n;f35:n;f36:n;f37:n;f38:n;f39:n;f40:n;f41:n;f42:n;f43:n;f44:n;f45:n;f46:n;f47:n;f48:n;f49:n;f50:n;f51:n;f52:n;f53:n;f54:n;f55:n;f56:n;f57:n;f58:n;f59:n;f60:n;f61:n;f62:n;f63:n;f64:n;f65:n;f66:n;f67:n;f68:n;f69:n;f70:n;f71:n;f72:n;f73:n;f74:n;f75:n;f76:n;f77:n;f78:n;f79:n;f80:n;f81:n;f82:n;f83:n;f84:n;f85:n;f86:n;f87:n;f88:n;f89:n;f90:n;f91:n;f92:n;f93:n;f94:n;f95:n;f96:n;f97:n;f98:n;f99:n;f100:n;f101:n;f102:n;f103:n;f104:n;f105:n;f106:n;f107:n;f108:n;f109:n;f110:n;f111:n;f112:n;f113:n;f114:n;f115:n;f116:n;f117:n;f118:n;f119:n;f120:n;f121:n;f122:n;f123:n;f124:n;f125:n;f126:n;f127:n;f128:n;f129:n;f130:n;f131:n;f132:n;f133:n;f134:n;f135:n;f136:n;f137:n;f138:n;f139:n;f140:n;f141:n;f142:n;f143:n;f144:n;f145:n;f146:n;f147:n;f148:n;f149:n} + +-- 150-field record literal, big enough to defeat the consecutive-slot +-- fast path. Read one field back at the far end of the record to +-- prove the per-field emission landed in the right slots. +hit-f140>n;r=big f0:0 f1:1 f2:2 f3:3 f4:4 f5:5 f6:6 f7:7 f8:8 f9:9 f10:10 f11:11 f12:12 f13:13 f14:14 f15:15 f16:16 f17:17 f18:18 f19:19 f20:20 f21:21 f22:22 f23:23 f24:24 f25:25 f26:26 f27:27 f28:28 f29:29 f30:30 f31:31 f32:32 f33:33 f34:34 f35:35 f36:36 f37:37 f38:38 f39:39 f40:40 f41:41 f42:42 f43:43 f44:44 f45:45 f46:46 f47:47 f48:48 f49:49 f50:50 f51:51 f52:52 f53:53 f54:54 f55:55 f56:56 f57:57 f58:58 f59:59 f60:60 f61:61 f62:62 f63:63 f64:64 f65:65 f66:66 f67:67 f68:68 f69:69 f70:70 f71:71 f72:72 f73:73 f74:74 f75:75 f76:76 f77:77 f78:78 f79:79 f80:80 f81:81 f82:82 f83:83 f84:84 f85:85 f86:86 f87:87 f88:88 f89:89 f90:90 f91:91 f92:92 f93:93 f94:94 f95:95 f96:96 f97:97 f98:98 f99:99 f100:100 f101:101 f102:102 f103:103 f104:104 f105:105 f106:106 f107:107 f108:108 f109:109 f110:110 f111:111 f112:112 f113:113 f114:114 f115:115 f116:116 f117:117 f118:118 f119:119 f120:120 f121:121 f122:122 f123:123 f124:124 f125:125 f126:126 f127:127 f128:128 f129:129 f130:130 f131:131 f132:132 f133:133 f134:134 f135:135 f136:136 f137:137 f138:138 f139:139 f140:140 f141:141 f142:142 f143:143 f144:144 f145:145 f146:146 f147:147 f148:148 f149:149;r.f140 + +-- Aggregate across multiple fields to confirm every slot is reachable. +sum-3>n;r=big f0:0 f1:1 f2:2 f3:3 f4:4 f5:5 f6:6 f7:7 f8:8 f9:9 f10:10 f11:11 f12:12 f13:13 f14:14 f15:15 f16:16 f17:17 f18:18 f19:19 f20:20 f21:21 f22:22 f23:23 f24:24 f25:25 f26:26 f27:27 f28:28 f29:29 f30:30 f31:31 f32:32 f33:33 f34:34 f35:35 f36:36 f37:37 f38:38 f39:39 f40:40 f41:41 f42:42 f43:43 f44:44 f45:45 f46:46 f47:47 f48:48 f49:49 f50:50 f51:51 f52:52 f53:53 f54:54 f55:55 f56:56 f57:57 f58:58 f59:59 f60:60 f61:61 f62:62 f63:63 f64:64 f65:65 f66:66 f67:67 f68:68 f69:69 f70:70 f71:71 f72:72 f73:73 f74:74 f75:75 f76:76 f77:77 f78:78 f79:79 f80:80 f81:81 f82:82 f83:83 f84:84 f85:85 f86:86 f87:87 f88:88 f89:89 f90:90 f91:91 f92:92 f93:93 f94:94 f95:95 f96:96 f97:97 f98:98 f99:99 f100:100 f101:101 f102:102 f103:103 f104:104 f105:105 f106:106 f107:107 f108:108 f109:109 f110:110 f111:111 f112:112 f113:113 f114:114 f115:115 f116:116 f117:117 f118:118 f119:119 f120:120 f121:121 f122:122 f123:123 f124:124 f125:125 f126:126 f127:127 f128:128 f129:129 f130:130 f131:131 f132:132 f133:133 f134:134 f135:135 f136:136 f137:137 f138:138 f139:139 f140:140 f141:141 f142:142 f143:143 f144:144 f145:145 f146:146 f147:147 f148:148 f149:149;+r.f0 +r.f75 r.f149 + +-- run: hit-f140 +-- out: 140 +-- run: sum-3 +-- out: 224 diff --git a/examples/large-record-with.ilo b/examples/large-record-with.ilo new file mode 100644 index 00000000..92d914c2 --- /dev/null +++ b/examples/large-record-with.ilo @@ -0,0 +1,18 @@ +-- Large `with` updates work at any size. When the consecutive-slot +-- layout used by OP_RECWITH would exceed the 255-reg frame budget, +-- the compiler falls back to OP_RECCOPY + per-update OP_RECSETFIELD +-- emission. Companion to large-record-literal.ilo for the update path. + +type big{f0:n;f1:n;f2:n;f3:n;f4:n;f5:n;f6:n;f7:n;f8:n;f9:n;f10:n;f11:n;f12:n;f13:n;f14:n;f15:n;f16:n;f17:n;f18:n;f19:n;f20:n;f21:n;f22:n;f23:n;f24:n;f25:n;f26:n;f27:n;f28:n;f29:n;f30:n;f31:n;f32:n;f33:n;f34:n;f35:n;f36:n;f37:n;f38:n;f39:n;f40:n;f41:n;f42:n;f43:n;f44:n;f45:n;f46:n;f47:n;f48:n;f49:n;f50:n;f51:n;f52:n;f53:n;f54:n;f55:n;f56:n;f57:n;f58:n;f59:n;f60:n;f61:n;f62:n;f63:n;f64:n;f65:n;f66:n;f67:n;f68:n;f69:n;f70:n;f71:n;f72:n;f73:n;f74:n;f75:n;f76:n;f77:n;f78:n;f79:n;f80:n;f81:n;f82:n;f83:n;f84:n;f85:n;f86:n;f87:n;f88:n;f89:n;f90:n;f91:n;f92:n;f93:n;f94:n;f95:n;f96:n;f97:n;f98:n;f99:n;f100:n;f101:n;f102:n;f103:n;f104:n;f105:n;f106:n;f107:n;f108:n;f109:n;f110:n;f111:n;f112:n;f113:n;f114:n;f115:n;f116:n;f117:n;f118:n;f119:n;f120:n;f121:n;f122:n;f123:n;f124:n;f125:n;f126:n;f127:n;f128:n;f129:n;f130:n;f131:n;f132:n;f133:n;f134:n;f135:n;f136:n;f137:n;f138:n;f139:n;f140:n;f141:n;f142:n;f143:n;f144:n;f145:n;f146:n;f147:n;f148:n;f149:n} + +-- All-zero record, then with-update every field to its index. Reads +-- f140 back to prove updates landed in the right slots. +upd-f140>n;r=big f0:0 f1:0 f2:0 f3:0 f4:0 f5:0 f6:0 f7:0 f8:0 f9:0 f10:0 f11:0 f12:0 f13:0 f14:0 f15:0 f16:0 f17:0 f18:0 f19:0 f20:0 f21:0 f22:0 f23:0 f24:0 f25:0 f26:0 f27:0 f28:0 f29:0 f30:0 f31:0 f32:0 f33:0 f34:0 f35:0 f36:0 f37:0 f38:0 f39:0 f40:0 f41:0 f42:0 f43:0 f44:0 f45:0 f46:0 f47:0 f48:0 f49:0 f50:0 f51:0 f52:0 f53:0 f54:0 f55:0 f56:0 f57:0 f58:0 f59:0 f60:0 f61:0 f62:0 f63:0 f64:0 f65:0 f66:0 f67:0 f68:0 f69:0 f70:0 f71:0 f72:0 f73:0 f74:0 f75:0 f76:0 f77:0 f78:0 f79:0 f80:0 f81:0 f82:0 f83:0 f84:0 f85:0 f86:0 f87:0 f88:0 f89:0 f90:0 f91:0 f92:0 f93:0 f94:0 f95:0 f96:0 f97:0 f98:0 f99:0 f100:0 f101:0 f102:0 f103:0 f104:0 f105:0 f106:0 f107:0 f108:0 f109:0 f110:0 f111:0 f112:0 f113:0 f114:0 f115:0 f116:0 f117:0 f118:0 f119:0 f120:0 f121:0 f122:0 f123:0 f124:0 f125:0 f126:0 f127:0 f128:0 f129:0 f130:0 f131:0 f132:0 f133:0 f134:0 f135:0 f136:0 f137:0 f138:0 f139:0 f140:0 f141:0 f142:0 f143:0 f144:0 f145:0 f146:0 f147:0 f148:0 f149:0;q=r with f0:0 f1:1 f2:2 f3:3 f4:4 f5:5 f6:6 f7:7 f8:8 f9:9 f10:10 f11:11 f12:12 f13:13 f14:14 f15:15 f16:16 f17:17 f18:18 f19:19 f20:20 f21:21 f22:22 f23:23 f24:24 f25:25 f26:26 f27:27 f28:28 f29:29 f30:30 f31:31 f32:32 f33:33 f34:34 f35:35 f36:36 f37:37 f38:38 f39:39 f40:40 f41:41 f42:42 f43:43 f44:44 f45:45 f46:46 f47:47 f48:48 f49:49 f50:50 f51:51 f52:52 f53:53 f54:54 f55:55 f56:56 f57:57 f58:58 f59:59 f60:60 f61:61 f62:62 f63:63 f64:64 f65:65 f66:66 f67:67 f68:68 f69:69 f70:70 f71:71 f72:72 f73:73 f74:74 f75:75 f76:76 f77:77 f78:78 f79:79 f80:80 f81:81 f82:82 f83:83 f84:84 f85:85 f86:86 f87:87 f88:88 f89:89 f90:90 f91:91 f92:92 f93:93 f94:94 f95:95 f96:96 f97:97 f98:98 f99:99 f100:100 f101:101 f102:102 f103:103 f104:104 f105:105 f106:106 f107:107 f108:108 f109:109 f110:110 f111:111 f112:112 f113:113 f114:114 f115:115 f116:116 f117:117 f118:118 f119:119 f120:120 f121:121 f122:122 f123:123 f124:124 f125:125 f126:126 f127:127 f128:128 f129:129 f130:130 f131:131 f132:132 f133:133 f134:134 f135:135 f136:136 f137:137 f138:138 f139:139 f140:140 f141:141 f142:142 f143:143 f144:144 f145:145 f146:146 f147:147 f148:148 f149:149;q.f140 + +-- Confirm the original is untouched after the with-update. +orig-untouched>n;r=big f0:0 f1:0 f2:0 f3:0 f4:0 f5:0 f6:0 f7:0 f8:0 f9:0 f10:0 f11:0 f12:0 f13:0 f14:0 f15:0 f16:0 f17:0 f18:0 f19:0 f20:0 f21:0 f22:0 f23:0 f24:0 f25:0 f26:0 f27:0 f28:0 f29:0 f30:0 f31:0 f32:0 f33:0 f34:0 f35:0 f36:0 f37:0 f38:0 f39:0 f40:0 f41:0 f42:0 f43:0 f44:0 f45:0 f46:0 f47:0 f48:0 f49:0 f50:0 f51:0 f52:0 f53:0 f54:0 f55:0 f56:0 f57:0 f58:0 f59:0 f60:0 f61:0 f62:0 f63:0 f64:0 f65:0 f66:0 f67:0 f68:0 f69:0 f70:0 f71:0 f72:0 f73:0 f74:0 f75:0 f76:0 f77:0 f78:0 f79:0 f80:0 f81:0 f82:0 f83:0 f84:0 f85:0 f86:0 f87:0 f88:0 f89:0 f90:0 f91:0 f92:0 f93:0 f94:0 f95:0 f96:0 f97:0 f98:0 f99:0 f100:0 f101:0 f102:0 f103:0 f104:0 f105:0 f106:0 f107:0 f108:0 f109:0 f110:0 f111:0 f112:0 f113:0 f114:0 f115:0 f116:0 f117:0 f118:0 f119:0 f120:0 f121:0 f122:0 f123:0 f124:0 f125:0 f126:0 f127:0 f128:0 f129:0 f130:0 f131:0 f132:0 f133:0 f134:0 f135:0 f136:0 f137:0 f138:0 f139:0 f140:0 f141:0 f142:0 f143:0 f144:0 f145:0 f146:0 f147:0 f148:0 f149:0;q=r with f0:0 f1:1 f2:2 f3:3 f4:4 f5:5 f6:6 f7:7 f8:8 f9:9 f10:10 f11:11 f12:12 f13:13 f14:14 f15:15 f16:16 f17:17 f18:18 f19:19 f20:20 f21:21 f22:22 f23:23 f24:24 f25:25 f26:26 f27:27 f28:28 f29:29 f30:30 f31:31 f32:32 f33:33 f34:34 f35:35 f36:36 f37:37 f38:38 f39:39 f40:40 f41:41 f42:42 f43:43 f44:44 f45:45 f46:46 f47:47 f48:48 f49:49 f50:50 f51:51 f52:52 f53:53 f54:54 f55:55 f56:56 f57:57 f58:58 f59:59 f60:60 f61:61 f62:62 f63:63 f64:64 f65:65 f66:66 f67:67 f68:68 f69:69 f70:70 f71:71 f72:72 f73:73 f74:74 f75:75 f76:76 f77:77 f78:78 f79:79 f80:80 f81:81 f82:82 f83:83 f84:84 f85:85 f86:86 f87:87 f88:88 f89:89 f90:90 f91:91 f92:92 f93:93 f94:94 f95:95 f96:96 f97:97 f98:98 f99:99 f100:100 f101:101 f102:102 f103:103 f104:104 f105:105 f106:106 f107:107 f108:108 f109:109 f110:110 f111:111 f112:112 f113:113 f114:114 f115:115 f116:116 f117:117 f118:118 f119:119 f120:120 f121:121 f122:122 f123:123 f124:124 f125:125 f126:126 f127:127 f128:128 f129:129 f130:130 f131:131 f132:132 f133:133 f134:134 f135:135 f136:136 f137:137 f138:138 f139:139 f140:140 f141:141 f142:142 f143:143 f144:144 f145:145 f146:146 f147:147 f148:148 f149:149;r.f140 + +-- run: upd-f140 +-- out: 140 +-- run: orig-untouched +-- out: 0 diff --git a/tests/regression_record_register_overflow.rs b/tests/regression_record_register_overflow.rs new file mode 100644 index 00000000..eb8815c3 --- /dev/null +++ b/tests/regression_record_register_overflow.rs @@ -0,0 +1,362 @@ +// Regression tests for record-literal and `with`-update register overflow. +// +// `Expr::Record` originally emitted a single OP_RECNEW that read field +// values from consecutive registers `a+1..a+n_fields`. `Expr::With` +// emitted OP_RECWITH with the same layout for the update values. Both +// required `next_reg + n <= 255`, so any record with more than ~127 +// fields, or a moderate one preceded by many locals, hit a hard +// `assert!` panic at compile time on the VM and Cranelift backends. +// The tree-walk interpreter was unaffected (no register file). +// +// The fix mirrors the OP_LISTNEW/OP_LISTAPPEND split from PR #237: +// when the contiguous-register layout would overflow, the compiler +// falls back to OP_RECNEW_EMPTY + per-field OP_RECSETFIELD for record +// literals, and OP_RECCOPY + per-update OP_RECSETFIELD for `with` +// expressions. Both fallbacks only need two registers (result + +// scratch) regardless of field count. +// +// These tests pin the cross-engine behaviour at sizes spanning both +// branches plus the many-leading-locals shape that matched the +// original repro. + +use std::process::Command; + +fn ilo() -> Command { + Command::new(env!("CARGO_BIN_EXE_ilo")) +} + +fn run(engine: &str, src: &str, func: &str) -> String { + let out = ilo() + .args([src, engine, func]) + .output() + .expect("failed to run ilo"); + assert!( + out.status.success(), + "ilo {engine} on `{func}` failed: stderr={}", + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8_lossy(&out.stdout).trim().to_string() +} + +/// Build a source string with a `big` type of `n` fields and a function +/// that constructs the record assigning `f` = `i`, then returns +/// `f`. +fn record_source(n: usize, probe: usize) -> String { + let type_fields: Vec = (0..n).map(|i| format!("f{i}:n")).collect(); + let inits: Vec = (0..n).map(|i| format!("f{i}:{i}")).collect(); + format!( + "type big{{{}}}\ngo>n;r=big {};r.f{}", + type_fields.join(";"), + inits.join(" "), + probe, + ) +} + +/// Source that bumps the register file with ~180 leading locals before +/// emitting a 60-field record literal. Matches the shape that surfaced +/// the bug in agent-written code where prior bindings had eaten most +/// of the 255-register budget. +fn record_leading_locals_source() -> String { + let locals: Vec = (0..180).map(|i| format!("a{i}=0")).collect(); + let n = 60; + let type_fields: Vec = (0..n).map(|i| format!("f{i}:n")).collect(); + let inits: Vec = (0..n).map(|i| format!("f{i}:{i}")).collect(); + format!( + "type big{{{}}}\ngo>n;{};r=big {};r.f50", + type_fields.join(";"), + locals.join(";"), + inits.join(" "), + ) +} + +/// Build a source string that exercises `with`: a `big` record of `n` +/// fields starts all-zero, then a single `with` updates every field to +/// its index. Reads back `f` to confirm the update landed. +fn with_source(n: usize, probe: usize) -> String { + let type_fields: Vec = (0..n).map(|i| format!("f{i}:n")).collect(); + let zeroes: Vec = (0..n).map(|i| format!("f{i}:0")).collect(); + let updates: Vec = (0..n).map(|i| format!("f{i}:{i}")).collect(); + format!( + "type big{{{}}}\ngo>n;r=big {};q=r with {};q.f{}", + type_fields.join(";"), + zeroes.join(" "), + updates.join(" "), + probe, + ) +} + +/// `with` companion to record_leading_locals_source. +fn with_leading_locals_source() -> String { + let locals: Vec = (0..180).map(|i| format!("a{i}=0")).collect(); + let n = 60; + let type_fields: Vec = (0..n).map(|i| format!("f{i}:n")).collect(); + let zeroes: Vec = (0..n).map(|i| format!("f{i}:0")).collect(); + let updates: Vec = (0..n).map(|i| format!("f{i}:{i}")).collect(); + format!( + "type big{{{}}}\ngo>n;{};r=big {};q=r with {};q.f50", + type_fields.join(";"), + locals.join(";"), + zeroes.join(" "), + updates.join(" "), + ) +} + +fn check_record(engine: &str, n: usize, probe: usize) { + let src = record_source(n, probe); + let out = run(engine, &src, "go"); + assert_eq!( + out, + probe.to_string(), + "{engine} record n={n} probe={probe}: expected `{probe}`, got {out:?}" + ); +} + +fn check_with(engine: &str, n: usize, probe: usize) { + let src = with_source(n, probe); + let out = run(engine, &src, "go"); + assert_eq!( + out, + probe.to_string(), + "{engine} with n={n} probe={probe}: expected `{probe}`, got {out:?}" + ); +} + +fn check_record_leading_locals(engine: &str) { + let src = record_leading_locals_source(); + let out = run(engine, &src, "go"); + assert_eq!( + out, "50", + "{engine}: expected `50` from 180-locals + 60-field record literal, got {out:?}" + ); +} + +fn check_with_leading_locals(engine: &str) { + let src = with_leading_locals_source(); + let out = run(engine, &src, "go"); + assert_eq!( + out, "50", + "{engine}: expected `50` from 180-locals + 60-field `with`, got {out:?}" + ); +} + +// Size sweep covers: small (fits fast path trivially), medium (still +// fast path), the largest n_fields that fits in a u8 (255 — actually +// 220 to stay within the canonical-order register pressure during +// compile_expr), and the cases that defeat the fast path. 150 is well +// past the 127-ish ceiling where the contiguous-layout assert fires. +const SIZES: &[usize] = &[1, 16, 60, 150, 220]; + +#[test] +fn record_size_sweep_tree() { + for &n in SIZES { + let probe = n - 1; + check_record("--run-tree", n, probe); + } +} + +#[test] +fn record_size_sweep_vm() { + for &n in SIZES { + let probe = n - 1; + check_record("--run-vm", n, probe); + } +} + +#[test] +#[cfg(feature = "cranelift")] +fn record_size_sweep_cranelift() { + for &n in SIZES { + let probe = n - 1; + check_record("--run-cranelift", n, probe); + } +} + +#[test] +fn with_size_sweep_tree() { + for &n in SIZES { + let probe = n - 1; + check_with("--run-tree", n, probe); + } +} + +#[test] +fn with_size_sweep_vm() { + for &n in SIZES { + let probe = n - 1; + check_with("--run-vm", n, probe); + } +} + +#[test] +#[cfg(feature = "cranelift")] +fn with_size_sweep_cranelift() { + for &n in SIZES { + let probe = n - 1; + check_with("--run-cranelift", n, probe); + } +} + +#[test] +fn record_leading_locals_tree() { + check_record_leading_locals("--run-tree"); +} + +#[test] +fn record_leading_locals_vm() { + check_record_leading_locals("--run-vm"); +} + +#[test] +#[cfg(feature = "cranelift")] +fn record_leading_locals_cranelift() { + check_record_leading_locals("--run-cranelift"); +} + +#[test] +fn with_leading_locals_tree() { + check_with_leading_locals("--run-tree"); +} + +#[test] +fn with_leading_locals_vm() { + check_with_leading_locals("--run-vm"); +} + +#[test] +#[cfg(feature = "cranelift")] +fn with_leading_locals_cranelift() { + check_with_leading_locals("--run-cranelift"); +} + +// Adjacent coverage: confirm a large `with` preserves untouched fields +// from the source record (the fallback path runs OP_RECCOPY which must +// clone_rc every field, not just memcpy raw bits). +#[test] +fn with_preserves_untouched_fields_vm() { + let n = 150; + let type_fields: Vec = (0..n).map(|i| format!("f{i}:n")).collect(); + let inits: Vec = (0..n).map(|i| format!("f{i}:{i}")).collect(); + // Update only f0 — every other field must survive intact. + let src = format!( + "type big{{{}}}\ngo>n;r=big {};q=r with f0:999;q.f140", + type_fields.join(";"), + inits.join(" "), + ); + let out = run("--run-vm", &src, "go"); + assert_eq!(out, "140"); +} + +#[test] +#[cfg(feature = "cranelift")] +fn with_preserves_untouched_fields_cranelift() { + let n = 150; + let type_fields: Vec = (0..n).map(|i| format!("f{i}:n")).collect(); + let inits: Vec = (0..n).map(|i| format!("f{i}:{i}")).collect(); + let src = format!( + "type big{{{}}}\ngo>n;r=big {};q=r with f0:999;q.f140", + type_fields.join(";"), + inits.join(" "), + ); + let out = run("--run-cranelift", &src, "go"); + assert_eq!(out, "140"); +} + +// Adjacent coverage: large `with` must NOT mutate the original record. +// The OP_RECCOPY fallback allocates fresh storage, then OP_RECSETFIELD +// mutates the new record only. +#[test] +fn with_does_not_mutate_original_vm() { + let n = 150; + let type_fields: Vec = (0..n).map(|i| format!("f{i}:n")).collect(); + let zeroes: Vec = (0..n).map(|i| format!("f{i}:0")).collect(); + let updates: Vec = (0..n).map(|i| format!("f{i}:{i}")).collect(); + let src = format!( + "type big{{{}}}\ngo>n;r=big {};q=r with {};r.f140", + type_fields.join(";"), + zeroes.join(" "), + updates.join(" "), + ); + let out = run("--run-vm", &src, "go"); + assert_eq!(out, "0", "original record should be untouched after with"); +} + +#[test] +#[cfg(feature = "cranelift")] +fn with_does_not_mutate_original_cranelift() { + let n = 150; + let type_fields: Vec = (0..n).map(|i| format!("f{i}:n")).collect(); + let zeroes: Vec = (0..n).map(|i| format!("f{i}:0")).collect(); + let updates: Vec = (0..n).map(|i| format!("f{i}:{i}")).collect(); + let src = format!( + "type big{{{}}}\ngo>n;r=big {};q=r with {};r.f140", + type_fields.join(";"), + zeroes.join(" "), + updates.join(" "), + ); + let out = run("--run-cranelift", &src, "go"); + assert_eq!(out, "0", "original record should be untouched after with"); +} + +// Adjacent coverage: large record literal containing heap field values +// (strings). Confirms clone_rc bookkeeping in OP_RECSETFIELD is correct +// across the fallback boundary. +#[test] +fn record_with_string_fields_vm() { + let n = 150; + let type_fields: Vec = (0..n) + .map(|i| { + if i % 2 == 0 { + format!("f{i}:t") + } else { + format!("f{i}:n") + } + }) + .collect(); + let inits: Vec = (0..n) + .map(|i| { + if i % 2 == 0 { + format!("f{i}:\"s{i}\"") + } else { + format!("f{i}:{i}") + } + }) + .collect(); + let src = format!( + "type big{{{}}}\ngo>t;r=big {};r.f140", + type_fields.join(";"), + inits.join(" "), + ); + let out = run("--run-vm", &src, "go"); + assert_eq!(out, "s140"); +} + +#[test] +#[cfg(feature = "cranelift")] +fn record_with_string_fields_cranelift() { + let n = 150; + let type_fields: Vec = (0..n) + .map(|i| { + if i % 2 == 0 { + format!("f{i}:t") + } else { + format!("f{i}:n") + } + }) + .collect(); + let inits: Vec = (0..n) + .map(|i| { + if i % 2 == 0 { + format!("f{i}:\"s{i}\"") + } else { + format!("f{i}:{i}") + } + }) + .collect(); + let src = format!( + "type big{{{}}}\ngo>t;r=big {};r.f140", + type_fields.join(";"), + inits.join(" "), + ); + let out = run("--run-cranelift", &src, "go"); + assert_eq!(out, "s140"); +}