Hojun-Cho/ww
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

selfhost+test: single-source-of-truth @tagscr scratch reservation (#38)

Browse Source 
Closes STATUS latent #1: @tagscr shared 24B reservation across the
four tagged-scratch sites (cgreturn, pushargsrev, cgindex
tagged-elem, pointer-rooted struct-field tagged write). Any fn that
needed >24B (e.g. slice-in-tagged-field 32B) silently overflowed
into the neighbor frame slot. Surfaced concretely as getopttest's
errortable wwstage exit 16 after #37 fixed the upstream gaps.

c.tagscrsz: i32 on the cgen struct is the single source of truth.
tagscrbump(c, need) in scanlocals raises the max across all 4
reservation sites and returns the frame delta. All emit sites
(cgreturn / pushargsrev / cgindex / cgwidentaggedstore pointer-
rooted) read c.tagscrsz instead of hardcoded 24. Mirrors the existing
cgwidentaggedstore precedent; @tagbase keeps its 8B scanseenmark
dedup (always 8B, correct).

Unmasked latent bug (now fixed): scanlocals's pointer-rooted struct-
field tagged-write detection uses localfindnode(c, base.str) to
resolve the *struct base. For `fn fill(h: *holder)`, h's scan-time
stub from scanseenmark had tnode=nil, so the @tagscr reservation
never fired. Pre-#38 the hardcoded 24B masked this; #38's correctly-
sized slot exposed it. cgfn's param scan loop now sets
c.locals.tnode = scanp.lhs after scanseenmark so localfindnode
resolves param types at scan time.

Test 714 (tagged_return_scratch): 4 rows × 2 stages = 8 fixtures.
Direct adjacency repro; match-arm field-by-field read; **mixed-
sizes-one-fn** (16B pushargsrev widen + 32B cgreturn widen in the
same body — pins the lockstep invariant that a sibling site can't
undersize the shared slot); call-site struct-payload widen. Row 3
specifically would regress if a future refactor ever forgets to
route an emit site through c.tagscrsz.

982 getopt_run green through both stages (was the original surface);
995 self_rebuild byte-id holds.
This commit is contained in:
Hojun-Cho
2026-05-16 13:45:33 +09:00
parent de3bd5cc3b
commit 28f36d84d8
9 changed files with 541 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files

108
selfhost/cmd/wwdump/main.combined.ww
View File

@@ -6408,7 +6408,7 @@ fn pushargsrev(c: *cgen, arg: *node, param: *node) i32 = {
let pname: str = rhsstructpayload(c, arg);
if (pname.len > 0) {
let ptype: *node = param.lhs;
let scroff: i32 = localadd(c, "@tagscr", 24, nil);
let scroff: i32 = localadd(c, "@tagscr", c.tagscrsz, nil);
emitline("\tXORQ\tAX, AX\n");
let zz: i32 = 0;
for (zz < widensz) {
@@ -8813,7 +8813,11 @@ fn cgwidentaggedstore(c: *cgen, dst: *node, src: *node,
emitline(", ");
emitoff(bspill: i64);
emitline("(BP)\n");
let scr: i32 = localadd(c, "@tagscr", slot_sz, nil);
// Same shared scratch — c.tagscrsz is the per-fn max across every
// reservation site (scanlocals); pinning to slot_sz here would
// undersize the slot if a sibling site (cgreturn, pushargsrev,
// cgindex) needed a larger one and fired second.
let scr: i32 = localadd(c, "@tagscr", c.tagscrsz, nil);
emitline("\tXORQ\tAX, AX\n");
let z: i32 = 0;
for (z < slot_sz) {
@@ -13054,7 +13058,7 @@ fn cgassign(c: *cgen, n: *node) void = {
if (istaggedtype(c, elemtn)) {
let slot_sz: i32 = slotsize(c, elemtn);
let scroff: i32 = localadd(c, "@tagscr",
24, nil);
c.tagscrsz, nil);
// Pre-zero scratch (matches push helper).
emitline("\tXORQ\tAX, AX\n");
let zz: i32 = 0;
@@ -15243,8 +15247,13 @@ fn cgreturn(c: *cgen, n: *node) void = {
};
if (needswiden) {
let rsz: i32 = slotsize(c, c.fnret);
// Use c.tagscrsz so the first @tagscr allocation in
// the fn lands a slot sized to the *max* across all
// uses (scanlocals bumped to rsz here). Hardcoding 24
// truncated 32B-slot returns and overwrote adjacent
// locals during the pre-zero loop (#38).
let scroff: i32 = localadd(c, "@tagscr",
24, nil);
c.tagscrsz, nil);
emitline("\tXORQ\tAX, AX\n");
let zz: i32 = 0;
for (zz < rsz) {
@@ -16376,6 +16385,28 @@ use typ;
use sym;
use strconv;
// tagscrbump — record that the body needs an @tagscr scratch slot of at
// least `need` bytes and return how many additional frame bytes that
// imposes. Each tagged-scratch reservation site calls this; the first
// raises c.tagscrsz from 0, later sites only grow it when they need
// more. Closes STATUS latent #1: pre-fix every site reserved a flat 24B
// and the slot under-allocated for any tagged-union with a 24B+ payload
// (e.g. `(void | err)` where `err` is 24B → slot_sz 32). Cgen-side
// emit (cgreturn / pushargsrev / cgindex / cgwidentaggedstore) reads
// c.tagscrsz to allocate the actual slot — scan + emit see the same
// number, so rob's "lockstep" invariant holds. The @tagscr lifetime is
// short-lived per use (zero, fill, copy out), and uses are sequential
// within a fn body, so sharing the max is safe.
fn tagscrbump(c: *cgen, need: i32) i32 = {
let n: i32 = need;
if (n < 8) { n = 8; };
if ((n & 7) != 0) { n = (n + 7) & ~7; };
if (n <= c.tagscrsz) { return 0; };
let delta: i32 = n - c.tagscrsz;
c.tagscrsz = n;
return delta;
};
//
// Recursively walks the body to count every local `let`. Each gets a
// slot sized by slotsize(typ); 8-byte default. Match-bindings + for-
@@ -16553,11 +16584,10 @@ fn scanlocals(c: *cgen, n: *node) i32 = {
};
return total;
};
// Tagged-arr/slice index store needs a 24B scratch slot
// (`@tagscr`) for cgwidentaggedstore to materialise the source
// in before copying to the element address. Reserved once per
// function (dedup'd via scanseenmark) regardless of how many
// tagged-arr stores the body contains.
// Tagged-arr/slice index store needs an @tagscr scratch slot for
// cgwidentaggedstore to materialise the source in before copying
// to the element address. Slot is shared per function via
// c.tagscrsz (raised to the largest element slot_sz seen).
if (n.kind == nkind.N_ASSIGN) {
let alhs: *node = n.lhs;
if (alhs != nil) {
@@ -16577,9 +16607,7 @@ fn scanlocals(c: *cgen, n: *node) i32 = {
if (bk == nkind.N_TPTR) { etn = btn.lhs; };
if (etn != nil) {
if (istaggedtype(c, etn)) {
if (!scanseenmark(c, "@tagscr")) {
total += 24;
};
total += tagscrbump(c, slotsize(c, etn));
};
};
};
@@ -16591,8 +16619,8 @@ fn scanlocals(c: *cgen, n: *node) i32 = {
// Tagged-union struct-field write: `s.f = v` or `(*p).f = v`
// where f is a tagged-union field. cgassign delegates to
// cgwidentaggedstore; for pointer-rooted dst the wrapper
// allocates @tagbase (8B) and @tagscr (slot_sz). Both names
// dedup with other tagged scratch users in the same function.
// allocates @tagbase (8B, fixed) and @tagscr (sized to the
// field's tagged slot). The @tagscr size feeds c.tagscrsz.
if (n.kind == nkind.N_ASSIGN) {
let alhs: *node = n.lhs;
if (alhs != nil) {
@@ -16625,9 +16653,7 @@ fn scanlocals(c: *cgen, n: *node) i32 = {
if (!scanseenmark(c, "@tagbase")) {
total += 8;
};
if (!scanseenmark(c, "@tagscr")) {
total += 24;
};
total += tagscrbump(c, slotsize(c, fi.tnode));
};
};
fi = nil;
@@ -16644,13 +16670,15 @@ fn scanlocals(c: *cgen, n: *node) i32 = {
};
};
// Tagged-union return with struct payload or tagged-subset
// source — cgreturn materialises in @tagscr then loads
// AX/DX/CX. Detect via the same rhsstructpayload predicate
// the cgen uses, so we only reserve when the cgen will
// actually emit a scratch-using path. `!void` / `!i32`
// aliases share N_STRUCTLIT shape but resolve to
// non-struct types — they fall through to scalar/str and
// don't need scratch.
// source — cgreturn materialises in @tagscr then loads AX/DX/
// CX/R8. Detect via the same rhsstructpayload predicate the
// cgen uses, so we only reserve when the cgen will actually
// emit a scratch-using path. `!void` / `!i32` aliases share
// N_STRUCTLIT shape but resolve to non-struct types — they
// fall through to scalar/str and don't need scratch. Slot is
// sized to the return type's slot_sz (was hardcoded 24, which
// truncated 32B slots — `(void | err24)` clobbered its own
// payload local; task #38).
if (n.kind == nkind.N_RETURN) {
if (c.fnret != nil) {
if (istaggedtype(c, c.fnret)) {
@@ -16670,9 +16698,7 @@ fn scanlocals(c: *cgen, n: *node) i32 = {
};
};
if (needs) {
if (!scanseenmark(c, "@tagscr")) {
total += 24;
};
total += tagscrbump(c, slotsize(c, c.fnret));
};
};
};
@@ -16749,9 +16775,7 @@ fn scanlocals(c: *cgen, n: *node) i32 = {
isidentstruct = true;
};
let _u: bool = isidentstruct;
if (!scanseenmark(c, "@tagscr")) {
total += 24;
};
total += tagscrbump(c, slotsize(c, pt));
};
};
};
@@ -17157,7 +17181,22 @@ fn cgfn(c: *cgen, fn_: *node) void = {
// Pure stack: lives at +BP(16+stkcursor*8); no
// local slot consumed. The reg cursor stays put.
}; };
scanseenmark(c, scanp.str);
// Record the param's tnode on the stub so scanlocals's
// `h.f = v` / `&h[i]` / etc. detection paths can
// resolve a *struct / *[]T / *T param through
// localfindnode rather than seeing tnode=nil and
// skipping the reservation. Latent pre-#38: the
// pointer-rooted struct-field tagged write
// (cgendecl.ww:264) never fired for `fn fill(h: *holder)
// { h.e = v; }` because the param stub had no type
// info, so @tagscr / @tagbase weren't counted in the
// frame. Emit-time localadd happened to fit pre-#38
// because the 24B hardcoded slot didn't collide with
// the 8B @tagbase neighbour, but a correctly-sized
// slot revealed the under-reservation.
if (!scanseenmark(c, scanp.str)) {
c.locals.tnode = scanp.lhs;
};
};
scanp = scanp.next;
};
@@ -17664,6 +17703,12 @@ type cgen = struct {
// `@vararg_sl_N` using this counter; cgcall resets and walks in
// the same order so the names line up at emission time.
varargseq: i32,
// Max @tagscr slot_sz across all reservation sites in the current
// function. scanlocals bumps; every emit-time `localadd("@tagscr",
// ...)` passes this same size so the first allocation lands a slot
// big enough for every later user. Single source of truth — pins
// rob's "scan + emit lockstep" invariant. Reset per cgfn.
tagscrsz: i32,
};
// Top-level mutable `let` registry. Mirrors cmd/w6c/cgen.c LetVar.
@@ -17685,6 +17730,7 @@ fn cgeninit(c: *cgen, a: *arena) void = {
c.lastwasreturn = 0;
c.labelseq = 0;
c.varargseq = 0;
c.tagscrsz = 0;
// Note: strlit_seq, strlits, ffis are *not* reset here; they
// persist across cgfn calls within one file. cgfile resets them
// at the start of each compilation unit.