diff --git a/Makefile b/Makefile index 596e77ec..d14f6deb 100644 --- a/Makefile +++ b/Makefile @@ -237,6 +237,7 @@ TESTS = $(BIN)/test_smoke $(BIN)/test_lex $(BIN)/test_parse $(BIN)/test_check \ $(BIN)/test_arrlit_str_full \ $(BIN)/test_redecl \ $(BIN)/test_struct_field_index \ + $(BIN)/test_tagged_return_scratch \ $(BIN)/test_param_shadow_mod \ $(BIN)/test_localoff_scope \ $(BIN)/test_cast_enum_movl \ @@ -470,6 +471,12 @@ $(BIN)/test_struct_field_index: test/wcc/713_struct_field_index.c \ $(LIB)/libwwrt.a | $(BIN) $(CC) $(CFLAGS) -o $@ $< +$(BIN)/test_tagged_return_scratch: test/wcc/714_tagged_return_scratch.c \ + $(BIN)/ww $(BIN)/w6c $(BIN)/w6a $(BIN)/w6l \ + $(BIN)/ww_ww $(BIN)/w6c_ww $(BIN)/w6a_ww $(BIN)/w6l_ww \ + $(LIB)/libwwrt.a | $(BIN) + $(CC) $(CFLAGS) -o $@ $< + $(BIN)/test_use_promote_alias: test/wcc/699_use_promote_alias.c \ $(BIN)/ww $(BIN)/w6c $(BIN)/w6a $(BIN)/w6l \ $(LIB)/libwwrt.a | $(BIN) diff --git a/selfhost/cmd/w6c/main.combined.ww b/selfhost/cmd/w6c/main.combined.ww index 0d9651b8..50b45a47 100644 --- a/selfhost/cmd/w6c/main.combined.ww +++ b/selfhost/cmd/w6c/main.combined.ww @@ -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. diff --git a/selfhost/cmd/wcc/cgen.ww b/selfhost/cmd/wcc/cgen.ww index 0709fc7b..a6d1040b 100644 --- a/selfhost/cmd/wcc/cgen.ww +++ b/selfhost/cmd/wcc/cgen.ww @@ -416,6 +416,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. @@ -437,6 +443,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. diff --git a/selfhost/cmd/wcc/cgendecl.ww b/selfhost/cmd/wcc/cgendecl.ww index e51c4323..6d98b38a 100644 --- a/selfhost/cmd/wcc/cgendecl.ww +++ b/selfhost/cmd/wcc/cgendecl.ww @@ -17,6 +17,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- @@ -194,11 +216,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) { @@ -218,9 +239,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)); }; }; }; @@ -232,8 +251,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) { @@ -266,9 +285,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; @@ -285,13 +302,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)) { @@ -311,9 +330,7 @@ fn scanlocals(c: *cgen, n: *node) i32 = { }; }; if (needs) { - if (!scanseenmark(c, "@tagscr")) { - total += 24; - }; + total += tagscrbump(c, slotsize(c, c.fnret)); }; }; }; @@ -390,9 +407,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)); }; }; }; @@ -798,7 +813,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; }; diff --git a/selfhost/cmd/wcc/cgenexpr.ww b/selfhost/cmd/wcc/cgenexpr.ww index 11240a3a..ca17c133 100644 --- a/selfhost/cmd/wcc/cgenexpr.ww +++ b/selfhost/cmd/wcc/cgenexpr.ww @@ -3451,7 +3451,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; diff --git a/selfhost/cmd/wcc/cgenstmt.ww b/selfhost/cmd/wcc/cgenstmt.ww index 9d04f6fd..1592e72d 100644 --- a/selfhost/cmd/wcc/cgenstmt.ww +++ b/selfhost/cmd/wcc/cgenstmt.ww @@ -191,8 +191,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) { diff --git a/selfhost/cmd/wcc/cgenutil.ww b/selfhost/cmd/wcc/cgenutil.ww index 4244ed0f..61f31c9c 100644 --- a/selfhost/cmd/wcc/cgenutil.ww +++ b/selfhost/cmd/wcc/cgenutil.ww @@ -165,7 +165,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) { @@ -2570,7 +2570,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) { diff --git a/selfhost/cmd/wwdump/main.combined.ww b/selfhost/cmd/wwdump/main.combined.ww index 9956b64d..3f2a1bd8 100644 --- a/selfhost/cmd/wwdump/main.combined.ww +++ b/selfhost/cmd/wwdump/main.combined.ww @@ -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. diff --git a/test/wcc/714_tagged_return_scratch.c b/test/wcc/714_tagged_return_scratch.c new file mode 100644 index 00000000..7e631ece --- /dev/null +++ b/test/wcc/714_tagged_return_scratch.c @@ -0,0 +1,303 @@ +/* + * 714_tagged_return_scratch — wwstage @tagscr scratch slot sized + * dynamically to the per-function max across all reservation sites. + * + * Pre-fix every site (cgreturn / pushargsrev / cgindex / cgassign-via + * cgwidentaggedstore) hardcoded 24B for @tagscr. A `(void | T)` return + * where T is 24B has slot_size = 32 (8B tag + 24B payload); the + * outer zero loop in cgreturn (cgenstmt.ww) then walked 32 bytes + * starting at the slot's BP offset, spilling 8B past the slot's end + * into the adjacent local — typically the very `T` being returned. + * The pre-zero stomped on its kind/flag bytes, so the subsequent + * field-by-field copy out delivered zeros in the AX/DX/CX/R8 return + * ABI's DX slot. cstage was unaffected (its inline mklabel(c, "tagscr") + * allocates a fresh slot of the requested size per call site). + * + * Fix (single source of truth via c.tagscrsz): + * - cgen.ww: cgen struct gains `tagscrsz: i32`; cgeninit resets. + * - cgendecl.ww: scanlocals's four @tagscr-reservation sites call + * tagscrbump(c, slotsize(c, T)) which raises c.tagscrsz to the + * max needed and returns the delta to add to the frame total. + * - cgenstmt.ww / cgenutil.ww / cgenexpr.ww: every emit-time + * localadd("@tagscr", _, nil) passes c.tagscrsz so the first + * allocation in the fn lands a slot sized for every later user. + * - cgenutil.ww (cgwidentaggedstore pointer-rooted): switches from + * local slot_sz to c.tagscrsz for the same lockstep reason. + * + * Closes STATUS latent #1 (`@tagscr` shared 24B reservation across all + * tagged scratch sites). Surfaced through getopttest's errortable + * scenario, where `tryparse` returns a 24B `error` value. + * + * Rows (runtime semantic round-trip; no asm byte-id — wwstage local + * layouts legitimately differ from cstage's and 995_self_rebuild + * covers cross-stage drift): + * + * row | what it pins + * ----------------------------+---------------------------------- + * ret_struct24_adjacent | (void | err24); local `e: err` + * | sits right above @tagscr; pre-zero + * | clobbered e+0..e+7 → kind/flag came + * | back zero. Post-fix: full payload + * | round-trips through the tagged + * | return. + * ret_struct24_via_match | scrutinee match arm reads kind + + * | flag + a + b from the case binding; + * | exercises both the cgreturn write + * | and the match-arm spill read. + * ret_mixed_sizes_one_fn | rob's lockstep pin — one fn body + * | holds TWO tagged-scratch sites at + * | different sizes: a 16B pushargsrev + * | widen (call into `(void | small)`) + * | and a 32B cgreturn widen (return + * | `(void | err24)`). scanlocals must + * | raise c.tagscrsz from 16→32 via + * | tagscrbump, and both emit sites must + * | dedupe to the same 32B slot. The + * | smaller pushargsrev site is the + * | FIRST '@' localadd (so it allocates + * | the slot); the larger cgreturn site + * | dedupes by name. If either side ever + * | regressed to a per-site size, the + * | 32B cgreturn zero loop would walk + * | past the 16B slot and clobber e+0.. + * | e+7 — same shape as row 1's adjacency + * | repro, but now staged through the + * | shared-slot path. + * ret_struct24_param_widen | call-site struct-payload widen into + * | a (void | err24) param uses the + * | same @tagscr; pushargsrev's slot + * | must hold all 32B. + */ +#include +#include +#include +#include +#include +#include + +static int +runwait(const char *cmd) +{ + int rc = system(cmd); + if (rc == -1) return -1; + if (WIFEXITED(rc)) return WEXITSTATUS(rc); + return -1; +} + +struct row { const char *label; const char *src; int want; }; + +static const struct row rows[] = { + /* 1. Direct repro of #38. Local `e: err` (24B) is declared before + * the return; scanlocals lays it out adjacent to @tagscr. Pre-fix + * (24B @tagscr) the outer zero loop wrote 32B and clobbered e+0. + * Returns 11 + 22 + 33 + 44 = 110. */ + { "ret_struct24_adjacent", + "type err = struct { kind: i32, flag: i32, a: i64, b: i64 };\n" + "fn outer() (void | err) = {\n" + " let e: err;\n" + " e.kind = 11; e.flag = 22; e.a = 33i64; e.b = 44i64;\n" + " return e;\n" + "};\n" + "fn main() i32 = {\n" + " let r: (void | err) = outer();\n" + " match (r) {\n" + " case void => return 99;\n" + " case let v: err => return v.kind + v.flag + (v.a: i32) + (v.b: i32);\n" + " };\n" + " return 0;\n" + "};\n", + 110 }, + + /* 2. Same shape but caller iterates fields through the match + * binding (no early-return-from-arm shortcut). Cross-checks that + * the match-arm spill reads each field at the expected offset + * after the tagged return ABI lands AX=tag, DX=kind+flag, + * CX=a, R8=b. Expected: 7 (kind=1) + 8 (flag=2) + 9 (a=3) + * + 10 (b=4) = 34. */ + { "ret_struct24_via_match", + "type err = struct { kind: i32, flag: i32, a: i64, b: i64 };\n" + "fn make() (void | err) = {\n" + " let e: err;\n" + " e.kind = 1; e.flag = 2; e.a = 3i64; e.b = 4i64;\n" + " return e;\n" + "};\n" + "fn main() i32 = {\n" + " let r: (void | err) = make();\n" + " let acc: i32 = 0;\n" + " match (r) {\n" + " case void => return 99;\n" + " case let v: err => {\n" + " acc += v.kind + 6;\n" + " acc += v.flag + 6;\n" + " acc += (v.a: i32) + 6;\n" + " acc += (v.b: i32) + 6;\n" + " };\n" + " };\n" + " return acc;\n" + "};\n", + 34 }, + + /* 3. Rob's lockstep pin: TWO tagged-scratch sites of different + * sizes in ONE fn body. `mixed` has a pushargsrev widen (16B + * slot, bare call into a `(void | small)` param) and a cgreturn + * widen (32B slot, returning `e: err`). + * + * Source order is chosen so the local layout pre-fix lands the + * cgreturn zero-loop overflow on the returned value's own bytes. + * `consume_small` returns void and is called bare (no let-bind), + * so @tagscr is the LAST localalloc and `e` sits immediately + * above it: + * + * localalloc s → -8 (8B) + * localalloc e → -32 (24B; e.kind/flag at -32, e.a/-24, e.b/-16) + * localadd @tagscr → -56 (pushargsrev hits first; pre-fix 24B slot) + * cgreturn @tagscr → dedupes to -56 + * cgreturn zero loop walks rsz=32 → writes -56,-48,-40,-32: + * -32 lands on e.kind/e.flag → both zero before + * cgwidentaggedstore reads e to fill scratch → AX/DX ABI + * ships kind=0 instead of 100. + * + * Post-fix: scanlocals raises c.tagscrsz from 16 → 32 via + * tagscrbump across the two sites; localadd allocates a 32B + * slot (-64), zero loop stays in bounds, e survives, main + * returns 100. Also pins the '@'-prefix dedup contract: both + * sites must see the same slot, not fork into separate + * allocations of their own per-site sizes. */ + { "ret_mixed_sizes_one_fn", + "type err = struct { kind: i32, flag: i32, a: i64, b: i64 };\n" + "type small = struct { v: i32 };\n" + "fn consume_small(r: (void | small)) void = {\n" + " match (r) {\n" + " case void => return;\n" + " case let v: small => return;\n" + " };\n" + "};\n" + "fn mixed() (void | err) = {\n" + " let s: small;\n" + " s.v = 7;\n" + " let e: err;\n" + " e.kind = 100; e.flag = 0; e.a = 0i64; e.b = 0i64;\n" + " consume_small(s);\n" + " return e;\n" + "};\n" + "fn main() i32 = {\n" + " let r: (void | err) = mixed();\n" + " match (r) {\n" + " case void => return 91;\n" + " case let v: err => return v.kind;\n" + " };\n" + " return 0;\n" + "};\n", + 100 }, + + /* 4. Call-site struct-payload widen: pass an err24 value to a fn + * whose param is (void | err24). pushargsrev materialises the + * payload in @tagscr (slot 32B) and pushes. Sister of row 1 on + * the call-arg side. Expected: 50 (kind) + 60 (flag) = 110. */ + { "ret_struct24_param_widen", + "type err = struct { kind: i32, flag: i32, a: i64, b: i64 };\n" + "fn consume(r: (void | err)) i32 = {\n" + " match (r) {\n" + " case void => return 91;\n" + " case let v: err => return v.kind + v.flag;\n" + " };\n" + " return 0;\n" + "};\n" + "fn main() i32 = {\n" + " let e: err;\n" + " e.kind = 50; e.flag = 60; e.a = 0i64; e.b = 0i64;\n" + " return consume(e);\n" + "};\n", + 110 }, +}; + +static int +run_driver(const char *driver, const struct row *r, int i) +{ + char src[64], tmpdir[64], cmd[1024]; + snprintf(src, sizeof src, "/tmp/wctrs_%d_%d.ww", getpid(), i); + snprintf(tmpdir, sizeof tmpdir, "/tmp/wctrs_%d_d_%d", getpid(), i); + + FILE *f = fopen(src, "wb"); + if (!f) return -1; + fputs(r->src, f); + fclose(f); + + mkdir(tmpdir, 0755); + snprintf(cmd, sizeof cmd, "cd %s && %s build %s", + tmpdir, driver, src); + if (runwait(cmd) != 0) { + fprintf(stderr, "row[%s]: build via %s failed\n", + r->label, driver); + unlink(src); rmdir(tmpdir); + return -1; + } + + const char *base = strrchr(src, '/'); + base = base ? base + 1 : src; + char outbin[128]; + snprintf(outbin, sizeof outbin, "%s/%s", tmpdir, base); + char *dot = strrchr(outbin, '.'); + if (dot && strcmp(dot, ".ww") == 0) *dot = '\0'; + int got = runwait(outbin); + + unlink(src); unlink(outbin); rmdir(tmpdir); + return got; +} + +int +main(void) +{ + const char *bin = getenv("BIN"); + if (!bin) bin = "out/bin"; + char absbin[512]; + if (bin[0] != '/') { + char cwd[256]; + if (getcwd(cwd, sizeof cwd) == NULL) return 1; + snprintf(absbin, sizeof absbin, "%s/%s", cwd, bin); + bin = absbin; + } + + char cdrv[640]; + snprintf(cdrv, sizeof cdrv, "%s/ww", bin); + char wdrv[640]; + snprintf(wdrv, sizeof wdrv, "%s/ww_ww", bin); + + struct { const char *name; const char *path; int gated_on_existence; } + drivers[] = { + { "cstage", cdrv, 0 }, + { "wwstage", wdrv, 1 }, + { NULL, NULL, 0 }, + }; + + int n = (int)(sizeof rows / sizeof rows[0]); + int total = 0, fail = 0; + + for (int d = 0; drivers[d].name; d++) { + if (drivers[d].gated_on_existence + && access(drivers[d].path, X_OK) != 0) { + fprintf(stderr, "tagged_return_scratch: skip %s (no %s)\n", + drivers[d].name, drivers[d].path); + continue; + } + for (int i = 0; i < n; i++) { + int got = run_driver(drivers[d].path, &rows[i], i); + total++; + if (got != rows[i].want) { + fprintf(stderr, + "tagged_return_scratch[%s] row[%s]: exit=%d want=%d\n", + drivers[d].name, rows[i].label, got, + rows[i].want); + fail++; + } + } + } + + if (fail) { + fprintf(stderr, + "tagged_return_scratch: %d/%d row(s) failed\n", fail, total); + return 1; + } + printf("tagged_return_scratch: %d/%d ok\n", total, total); + return 0; +}