diff --git a/selfhost/cmd/w6c/main.combined.ww b/selfhost/cmd/w6c/main.combined.ww index 50b45a47..4acc0c13 100644 --- a/selfhost/cmd/w6c/main.combined.ww +++ b/selfhost/cmd/w6c/main.combined.ww @@ -8443,6 +8443,18 @@ fn rhstargetname(c: *cgen, rhs: *node) str = { fn taggedvariantindex(c: *cgen, tagged: *node, rhs: *node) i32 = { if (tagged == nil) { return -1; }; if (rhs == nil) { return -1; }; + // Alias-unwrap: wwstage has no typed AST, so an aliased tagged + // return (`type ft = (i64|str|bool); fn f() ft = ...`) reaches + // here as N_TNAME("ft"), not N_TTAGGED. flatvariantidx and the + // fallback both gate on N_TTAGGED → -1 → caller maps to 0, + // silently emitting `MOVQ $0, AX` for every non-leading variant. + // Cstage's check.c canonicalizes N_TNAME → underlying upfront; + // every wwstage cgen consumer of a type-bearing node has to + // remember this step itself. TODO(#11): a wwstage check pass + // between parse and cgen would replace the per-site unwrap with + // a single canonicalization. Same shape of fix as nodeisstr. + let resolved: *node = resolvetagged(c, tagged); + if (resolved != nil) { tagged = resolved; }; let wantname: str = rhstargetname(c, rhs); if (wantname.len > 0) { let r: i32 = flatvariantidx(c, tagged, wantname); diff --git a/selfhost/cmd/wcc/cgenutil.ww b/selfhost/cmd/wcc/cgenutil.ww index 61f31c9c..f6a0e213 100644 --- a/selfhost/cmd/wcc/cgenutil.ww +++ b/selfhost/cmd/wcc/cgenutil.ww @@ -2200,6 +2200,18 @@ fn rhstargetname(c: *cgen, rhs: *node) str = { fn taggedvariantindex(c: *cgen, tagged: *node, rhs: *node) i32 = { if (tagged == nil) { return -1; }; if (rhs == nil) { return -1; }; + // Alias-unwrap: wwstage has no typed AST, so an aliased tagged + // return (`type ft = (i64|str|bool); fn f() ft = ...`) reaches + // here as N_TNAME("ft"), not N_TTAGGED. flatvariantidx and the + // fallback both gate on N_TTAGGED → -1 → caller maps to 0, + // silently emitting `MOVQ $0, AX` for every non-leading variant. + // Cstage's check.c canonicalizes N_TNAME → underlying upfront; + // every wwstage cgen consumer of a type-bearing node has to + // remember this step itself. TODO(#11): a wwstage check pass + // between parse and cgen would replace the per-site unwrap with + // a single canonicalization. Same shape of fix as nodeisstr. + let resolved: *node = resolvetagged(c, tagged); + if (resolved != nil) { tagged = resolved; }; let wantname: str = rhstargetname(c, rhs); if (wantname.len > 0) { let r: i32 = flatvariantidx(c, tagged, wantname); diff --git a/selfhost/cmd/wwdump/main.combined.ww b/selfhost/cmd/wwdump/main.combined.ww index 3f2a1bd8..85d9c22a 100644 --- a/selfhost/cmd/wwdump/main.combined.ww +++ b/selfhost/cmd/wwdump/main.combined.ww @@ -8443,6 +8443,18 @@ fn rhstargetname(c: *cgen, rhs: *node) str = { fn taggedvariantindex(c: *cgen, tagged: *node, rhs: *node) i32 = { if (tagged == nil) { return -1; }; if (rhs == nil) { return -1; }; + // Alias-unwrap: wwstage has no typed AST, so an aliased tagged + // return (`type ft = (i64|str|bool); fn f() ft = ...`) reaches + // here as N_TNAME("ft"), not N_TTAGGED. flatvariantidx and the + // fallback both gate on N_TTAGGED → -1 → caller maps to 0, + // silently emitting `MOVQ $0, AX` for every non-leading variant. + // Cstage's check.c canonicalizes N_TNAME → underlying upfront; + // every wwstage cgen consumer of a type-bearing node has to + // remember this step itself. TODO(#11): a wwstage check pass + // between parse and cgen would replace the per-site unwrap with + // a single canonicalization. Same shape of fix as nodeisstr. + let resolved: *node = resolvetagged(c, tagged); + if (resolved != nil) { tagged = resolved; }; let wantname: str = rhstargetname(c, rhs); if (wantname.len > 0) { let r: i32 = flatvariantidx(c, tagged, wantname); diff --git a/test/wcc/707_cgreturn_variant_zero.c b/test/wcc/707_cgreturn_variant_zero.c index 0cc92555..e2ebf208 100644 --- a/test/wcc/707_cgreturn_variant_zero.c +++ b/test/wcc/707_cgreturn_variant_zero.c @@ -1,9 +1,10 @@ /* * 707_cgreturn_variant_zero — tagged-return ABI variant-widen zero-pad - * for unused AX/DX/CX/R8 words (task #18). + * for unused AX/DX/CX/R8 words (task #18) plus aliased-tagged variant- + * index lookup (task #20). * - * Pre-fix: cgreturn's `!istagged && !isstruct` variant-widen arm only - * filled the registers a given variant actually uses (scalar → DX; + * Pre-fix (#18): cgreturn's `!istagged && !isstruct` variant-widen arm + * only filled the registers a given variant actually uses (scalar → DX; * str → DX, CX; slice → DX, CX, R8). The unused AX/CX/R8 words were * left holding whatever the caller's last write parked there. The * receive side (cg_widen_tagged_store call-source arm) writes @@ -19,13 +20,23 @@ * but an index expression in any straight-line code triggers it * just as well. * - * Fix: in cgreturn's variant-widen arm, after the variant-specific - * register shuffle and before the tag-into-AX, emit `MOVQ $0, CX` / - * `MOVQ $0, R8` for variants that don't naturally fill those words, - * conditional on the dst tagged-union slot size. Symmetric across - * cstage cgen.c and wwstage cgenstmt.ww. + * Fix (#18): in cgreturn's variant-widen arm, after the variant- + * specific register shuffle and before the tag-into-AX, emit + * `MOVQ $0, CX` / `MOVQ $0, R8` for variants that don't naturally + * fill those words, conditional on the dst tagged-union slot size. + * Symmetric across cstage cgen.c and wwstage cgenstmt.ww. * - * What this test pins (runtime only): + * Pre-fix (#20): wwstage's `taggedvariantindex` checked `tagged.kind + * == N_TTAGGED` directly. For aliased return types + * (`type ft = (i64|str|bool); fn f() ft = ...`) c.fnret is N_TNAME, + * so the lookup punted to -1 → caller mapped to 0, silently emitting + * `MOVQ $0, AX` for every non-leading variant. Cstage was already + * correct (check.c resolves N_TNAME → underlying upfront). + * + * Fix (#20): resolve `tagged` via resolvetagged() (alias + TBANG + * unwrap) at entry to taggedvariantindex. Wwstage-only. + * + * What this test pins: * - Every scalar-variant arm of a 24B tagged-union return zeroes * slot+16 (CX) regardless of caller priming. * - The str variant of a 24B slot still propagates .len correctly @@ -33,14 +44,14 @@ * zero for the 24B case, leaving CX/.len intact). * - Cross-shape probes: straight-line array-index call, in-loop * body call, nested-loop inner-body call. - * - * Asm byte-identity is NOT diffed here: wwstage has a pre-existing - * gap in taggedvariantindex's str/bool-N_IDENT lookup (returns 0 - * regardless of declared variant), so non-i64 variant rows produce - * a divergent `MOVQ $tag, AX`. The fix in this commit IS symmetric - * (the new `MOVQ $0, CX` / `MOVQ $0, R8` lines match byte-for-byte - * across stages); global bootstrap byte-identity at test 995 - * covers it. The wwstage variant-index gap is tracked separately. + * - Aliased tagged-return + N_IDENT source emits the declared- + * position variant tag (#20). Rows 7-9 pin tag=0 / tag=1 / + * tag=2 explicitly so a future variant reorder doesn't + * silently land back on the coincidentally-correct tag=0. + * - Cstage and wwstage emit byte-identical asm for every row, + * post #18 + #20 fixes. The bootstrap byte-identity invariant + * (995_self_rebuild) is the global guard; per-row diffs here + * surface focused regressions in cgreturn / taggedvariantindex. * * Test rows use `[N]i64` array indexing as the CX primer (well- * supported on both stages) to dodge a pre-existing cstage/wwstage @@ -194,6 +205,48 @@ static const struct row rows[] = { " return bad;\n" "};\n", 0 }, + /* 7-9: task #20 variant-index pins. Aliased tagged return plus + * N_IDENT source on each variant arm — probes slot+0 (the tag + * word). Pre-#20 wwstage's taggedvariantindex saw c.fnret as + * N_TNAME("ft") and short-circuited to -1 → tag = 0 for every + * row. Post-#20 the resolvetagged unwrap at fn entry surfaces + * the underlying N_TTAGGED so the declared-position index is + * emitted. Pinning all three positions (0/1/2) catches a future + * variant-reorder regression that would otherwise hide behind + * row 7's coincidentally-correct tag=0. */ + { "aliased_tagged_aski64_idx0", + "type ft = (i64 | str | bool);\n" + "fn aski64(v: i64) ft = { return v; };\n" + "fn main() i32 = {\n" + " let v: i64 = 7i64;\n" + " let a: ft = aski64(v);\n" + " let p: u64 = (&a): u64;\n" + " let pt: *i64 = p: *i64;\n" + " return (*pt): i32;\n" + "};\n", + 0 }, + { "aliased_tagged_askstr_idx1", + "type ft = (i64 | str | bool);\n" + "fn askstr(s: str) ft = { return s; };\n" + "fn main() i32 = {\n" + " let s: str = \"world\";\n" + " let a: ft = askstr(s);\n" + " let p: u64 = (&a): u64;\n" + " let pt: *i64 = p: *i64;\n" + " return (*pt): i32;\n" + "};\n", + 1 }, + { "aliased_tagged_askbool_idx2", + "type ft = (i64 | str | bool);\n" + "fn askbool(b: bool) ft = { return b; };\n" + "fn main() i32 = {\n" + " let b: bool = true;\n" + " let a: ft = askbool(b);\n" + " let p: u64 = (&a): u64;\n" + " let pt: *i64 = p: *i64;\n" + " return (*pt): i32;\n" + "};\n", + 2 }, }; static int @@ -230,6 +283,59 @@ run_driver(const char *driver, const struct row *r, int i) return got; } +/* asm_byte_identical — generate .s via cstage's w6c and wwstage's w6c_ww + * and diff. Post #18 + #20 fixes the per-row asm matches byte-for-byte + * across stages; the bootstrap byte-identity (995_self_rebuild) covers + * the broader cross-stage drift surface globally. */ +static int +asm_byte_identical(const char *bin, const struct row *r, int i) +{ + char src[64], cs[64], ws[64], cmd[1024]; + snprintf(src, sizeof src, "/tmp/wcrv_asm_%d_%d.ww", getpid(), i); + snprintf(cs, sizeof cs, "/tmp/wcrv_asm_%d_%d_c.s", getpid(), i); + snprintf(ws, sizeof ws, "/tmp/wcrv_asm_%d_%d_w.s", getpid(), i); + + FILE *f = fopen(src, "wb"); + if (!f) return -1; + fputs(r->src, f); + fclose(f); + + snprintf(cmd, sizeof cmd, "%s/w6c -o %s %s 2>/dev/null", bin, cs, src); + if (runwait(cmd) != 0) { + fprintf(stderr, "row[%s]: w6c errored\n", r->label); + unlink(src); + return -1; + } + snprintf(cmd, sizeof cmd, "%s/w6c_ww -o %s %s 2>/dev/null", + bin, ws, src); + if (runwait(cmd) != 0) { + fprintf(stderr, "row[%s]: w6c_ww errored\n", r->label); + unlink(src); unlink(cs); + return -1; + } + + FILE *fc = fopen(cs, "rb"); + FILE *fw = fopen(ws, "rb"); + int rc = 0; + if (!fc || !fw) { + rc = -1; + } else { + for (;;) { + int a = fgetc(fc); + int b = fgetc(fw); + if (a != b) { rc = -1; break; } + if (a == EOF) break; + } + } + if (fc) fclose(fc); + if (fw) fclose(fw); + if (rc != 0) + fprintf(stderr, "row[%s]: cstage vs wwstage asm differs\n", + r->label); + unlink(src); unlink(cs); unlink(ws); + return rc; +} + int main(void) { @@ -278,6 +384,15 @@ main(void) } } + /* Asm byte-identity diff, only when both stages exist. */ + if (access(wdrv, X_OK) == 0) { + for (int i = 0; i < n; i++) { + total++; + if (asm_byte_identical(bin, &rows[i], i) != 0) + fail++; + } + } + if (fail) { fprintf(stderr, "cgreturn_variant_zero: %d/%d fixtures failed\n", fail, total);