diff --git a/Makefile b/Makefile index 9015c97b..6c6d0cc2 100644 --- a/Makefile +++ b/Makefile @@ -338,6 +338,7 @@ TESTS = $(BIN)/test_smoke $(BIN)/test_lex $(BIN)/test_parse $(BIN)/test_check \ $(BIN)/test_def_mangle_run \ $(BIN)/test_arr_u16_store_run \ $(BIN)/test_arr_module_index_run \ + $(BIN)/test_arr_float_call_index_run \ $(BIN)/test_f64cgen_run \ $(BIN)/test_f64crossmod_run \ $(BIN)/test_tuprecv_run \ @@ -1141,6 +1142,11 @@ $(BIN)/test_arr_module_index_run: test/wcc/915_arr_module_index_run.c $(BIN)/ww $(LIB)/libwwrt.a | $(BIN) $(CC) $(CFLAGS) -o $@ $< +$(BIN)/test_arr_float_call_index_run: test/wcc/916_arr_float_call_index_run.c \ + $(BIN)/ww $(BIN)/w6c $(BIN)/w6c_ww $(BIN)/w6a $(BIN)/w6l \ + $(LIB)/libwwrt.a | $(BIN) + $(CC) $(CFLAGS) -o $@ $< + $(BIN)/test_f64cgen_run: test/wcc/951_f64cgen_run.c $(BIN)/ww $(BIN)/w6c \ $(BIN)/w6a $(BIN)/w6l $(LIB)/libwwrt.a | $(BIN) $(CC) $(CFLAGS) -o $@ $< diff --git a/cmd/w6c/cgen.c b/cmd/w6c/cgen.c index 6e396a89..71820c01 100644 --- a/cmd/w6c/cgen.c +++ b/cmd/w6c/cgen.c @@ -3951,7 +3951,20 @@ cgexpr(Cg *c, Node *n, Local *locals) ins1(c, A_PUSHQ, areg(D_CX)); /* cap */ ins1(c, A_PUSHQ, areg(D_BX)); /* len */ } - ins1(c, A_PUSHQ, areg(D_AX)); + /* Float element: spill X0 (not AX — AX is junk + * for floats) across the idx/base eval. A call- + * index (`a[geti()]=v`) clobbers X0 and would + * otherwise lose the value. Mirrors the *p=v + * float deref store at cgen.c:4187 (#125). */ + int sp_isfloat = type_isfloat(esub); + int sp_mov = sp_isfloat + ? (type_isf32(esub) ? A_MOVSS : A_MOVSD) : 0; + if (sp_isfloat) { + ins2(c, A_SUBQ, aimm(8), areg(D_SP)); + ins2(c, sp_mov, areg(D_X0), amem(D_SP, 0)); + } else { + ins1(c, A_PUSHQ, areg(D_AX)); + } cgexpr(c, n->lhs->rhs, locals); /* idx → AX */ if (esz > 1) { ins2(c, A_MOVQ, aimm(esz), areg(D_CX)); @@ -3993,7 +4006,15 @@ cgexpr(Cg *c, Node *n, Local *locals) } ins1(c, A_POPQ, areg(D_AX)); /* scaled idx */ ins2(c, A_ADDQ, areg(D_AX), areg(D_BX)); - ins1(c, A_POPQ, areg(D_AX)); /* value (ptr if str) */ + /* Reload value: float reloads X0 from the spill + * slot; non-float pops AX. Twin of the value-spill + * site above (#125). */ + if (sp_isfloat) { + ins2(c, sp_mov, amem(D_SP, 0), areg(D_X0)); + ins2(c, A_ADDQ, aimm(8), areg(D_SP)); + } else { + ins1(c, A_POPQ, areg(D_AX)); /* value (ptr if str) */ + } if (elem_is_str || elem_is_slice) { /* str/slice: store ptr/len/cap (#1/Phase 3, #7). */ ins2(c, A_MOVQ, areg(D_AX), amem(D_BX, 0)); @@ -4008,12 +4029,10 @@ cgexpr(Cg *c, Node *n, Local *locals) * CVTSD2SS narrowing only touches X0 — the AX path * below would store the raw double low-bits (garbage * for f32). Float-ness from esub, mirroring the read - * side at cgen.c:6423 (#122). X0 survives the index/ - * base eval only for literal/local-var indices; a - * call-index (`a[geti()]=v`) clobbers X0 and loses the - * value — this path does not spill X0 across the eval, - * unlike the *p=v float deref store above. Deferred to - * #125. */ + * side at cgen.c:6423 (#122). #125: the value-spill + * pair above keeps X0 live across the idx/base eval + * so this MOVSS/MOVSD is correct even on call-index + * shapes. */ if (type_isfloat(esub)) { int mov = type_isf32(esub) ? A_MOVSS : A_MOVSD; ins2(c, mov, areg(D_X0), amem(D_BX, 0)); diff --git a/selfhost/cmd/w6c/main.combined.ww b/selfhost/cmd/w6c/main.combined.ww index b0cdccb8..4b77885b 100644 --- a/selfhost/cmd/w6c/main.combined.ww +++ b/selfhost/cmd/w6c/main.combined.ww @@ -19001,7 +19001,22 @@ fn cgassign(c: *cgen, n: *node) void = { emitline("\tPUSHQ\tCX\n"); emitline("\tPUSHQ\tBX\n"); }; - emitline("\tPUSHQ\tAX\n"); + // Float element: spill X0 (not AX — AX is junk for + // floats) across the idx/base eval. A call-index + // (a[geti()]=v) clobbers X0 and would otherwise lose + // the value. Mirrors the *p=v float deref store + // twin in cgassign (#125). + let spisfloat: bool = isfloattype(c, elemtn); + let spmov: str = "MOVSD"; + if (isf32type(c, elemtn)) { spmov = "MOVSS"; }; + if (spisfloat) { + emitline("\tSUBQ\t$8, SP\n"); + emitline("\t"); + emitline(spmov); + emitline("\tX0, (SP)\n"); + } else { + emitline("\tPUSHQ\tAX\n"); + }; cgexpr(c, idx); // idx → AX if (esz > 1) { emitline("\tMOVQ\t$"); @@ -19039,7 +19054,17 @@ fn cgassign(c: *cgen, n: *node) void = { };};};}; emitline("\tPOPQ\tAX\n"); // scaled idx emitline("\tADDQ\tAX, BX\n"); - emitline("\tPOPQ\tAX\n"); // value + // Reload value: float reloads X0 from the spill slot; + // non-float pops AX. Twin of the value-spill site + // above (#125). + if (spisfloat) { + emitline("\t"); + emitline(spmov); + emitline("\t(SP), X0\n"); + emitline("\tADDQ\t$8, SP\n"); + } else { + emitline("\tPOPQ\tAX\n"); // value + }; // str/slice: pop the saved len + cap and store // all three words. Kind-gate, not size — see the // spill site above (#1/Phase 3, #7/754). @@ -19052,11 +19077,9 @@ fn cgassign(c: *cgen, n: *node) void = { return; }; // float element → store FROM X0 (MOVSS/MOVSD): cgexpr - // left the value in X0. X0 survives the index/base - // eval only for literal/local-var indices; a call- - // index (a[geti()]=v) clobbers X0 and loses the value - // — this path does not spill X0 across the eval, - // unlike the *p=v float deref store. Deferred to #125. + // left the value in X0, and the value-spill pair + // above keeps X0 live across the idx/base eval so + // a call-index (a[geti()]=v) doesn't lose it (#125). // For f32 the #104 CVTSD2SS narrowing only touches X0, // so the AX store below would write raw double low- // bits, garbage for f32 (#122, mirrors cstage cgen.c diff --git a/selfhost/cmd/wcc/cgenexpr.ww b/selfhost/cmd/wcc/cgenexpr.ww index 9f7cabcf..aefd69da 100644 --- a/selfhost/cmd/wcc/cgenexpr.ww +++ b/selfhost/cmd/wcc/cgenexpr.ww @@ -4382,7 +4382,22 @@ fn cgassign(c: *cgen, n: *node) void = { emitline("\tPUSHQ\tCX\n"); emitline("\tPUSHQ\tBX\n"); }; - emitline("\tPUSHQ\tAX\n"); + // Float element: spill X0 (not AX — AX is junk for + // floats) across the idx/base eval. A call-index + // (a[geti()]=v) clobbers X0 and would otherwise lose + // the value. Mirrors the *p=v float deref store + // twin in cgassign (#125). + let spisfloat: bool = isfloattype(c, elemtn); + let spmov: str = "MOVSD"; + if (isf32type(c, elemtn)) { spmov = "MOVSS"; }; + if (spisfloat) { + emitline("\tSUBQ\t$8, SP\n"); + emitline("\t"); + emitline(spmov); + emitline("\tX0, (SP)\n"); + } else { + emitline("\tPUSHQ\tAX\n"); + }; cgexpr(c, idx); // idx → AX if (esz > 1) { emitline("\tMOVQ\t$"); @@ -4420,7 +4435,17 @@ fn cgassign(c: *cgen, n: *node) void = { };};};}; emitline("\tPOPQ\tAX\n"); // scaled idx emitline("\tADDQ\tAX, BX\n"); - emitline("\tPOPQ\tAX\n"); // value + // Reload value: float reloads X0 from the spill slot; + // non-float pops AX. Twin of the value-spill site + // above (#125). + if (spisfloat) { + emitline("\t"); + emitline(spmov); + emitline("\t(SP), X0\n"); + emitline("\tADDQ\t$8, SP\n"); + } else { + emitline("\tPOPQ\tAX\n"); // value + }; // str/slice: pop the saved len + cap and store // all three words. Kind-gate, not size — see the // spill site above (#1/Phase 3, #7/754). @@ -4433,11 +4458,9 @@ fn cgassign(c: *cgen, n: *node) void = { return; }; // float element → store FROM X0 (MOVSS/MOVSD): cgexpr - // left the value in X0. X0 survives the index/base - // eval only for literal/local-var indices; a call- - // index (a[geti()]=v) clobbers X0 and loses the value - // — this path does not spill X0 across the eval, - // unlike the *p=v float deref store. Deferred to #125. + // left the value in X0, and the value-spill pair + // above keeps X0 live across the idx/base eval so + // a call-index (a[geti()]=v) doesn't lose it (#125). // For f32 the #104 CVTSD2SS narrowing only touches X0, // so the AX store below would write raw double low- // bits, garbage for f32 (#122, mirrors cstage cgen.c diff --git a/selfhost/cmd/wwdump/main.combined.ww b/selfhost/cmd/wwdump/main.combined.ww index f555a65c..8d32f5e6 100644 --- a/selfhost/cmd/wwdump/main.combined.ww +++ b/selfhost/cmd/wwdump/main.combined.ww @@ -19001,7 +19001,22 @@ fn cgassign(c: *cgen, n: *node) void = { emitline("\tPUSHQ\tCX\n"); emitline("\tPUSHQ\tBX\n"); }; - emitline("\tPUSHQ\tAX\n"); + // Float element: spill X0 (not AX — AX is junk for + // floats) across the idx/base eval. A call-index + // (a[geti()]=v) clobbers X0 and would otherwise lose + // the value. Mirrors the *p=v float deref store + // twin in cgassign (#125). + let spisfloat: bool = isfloattype(c, elemtn); + let spmov: str = "MOVSD"; + if (isf32type(c, elemtn)) { spmov = "MOVSS"; }; + if (spisfloat) { + emitline("\tSUBQ\t$8, SP\n"); + emitline("\t"); + emitline(spmov); + emitline("\tX0, (SP)\n"); + } else { + emitline("\tPUSHQ\tAX\n"); + }; cgexpr(c, idx); // idx → AX if (esz > 1) { emitline("\tMOVQ\t$"); @@ -19039,7 +19054,17 @@ fn cgassign(c: *cgen, n: *node) void = { };};};}; emitline("\tPOPQ\tAX\n"); // scaled idx emitline("\tADDQ\tAX, BX\n"); - emitline("\tPOPQ\tAX\n"); // value + // Reload value: float reloads X0 from the spill slot; + // non-float pops AX. Twin of the value-spill site + // above (#125). + if (spisfloat) { + emitline("\t"); + emitline(spmov); + emitline("\t(SP), X0\n"); + emitline("\tADDQ\t$8, SP\n"); + } else { + emitline("\tPOPQ\tAX\n"); // value + }; // str/slice: pop the saved len + cap and store // all three words. Kind-gate, not size — see the // spill site above (#1/Phase 3, #7/754). @@ -19052,11 +19077,9 @@ fn cgassign(c: *cgen, n: *node) void = { return; }; // float element → store FROM X0 (MOVSS/MOVSD): cgexpr - // left the value in X0. X0 survives the index/base - // eval only for literal/local-var indices; a call- - // index (a[geti()]=v) clobbers X0 and loses the value - // — this path does not spill X0 across the eval, - // unlike the *p=v float deref store. Deferred to #125. + // left the value in X0, and the value-spill pair + // above keeps X0 live across the idx/base eval so + // a call-index (a[geti()]=v) doesn't lose it (#125). // For f32 the #104 CVTSD2SS narrowing only touches X0, // so the AX store below would write raw double low- // bits, garbage for f32 (#122, mirrors cstage cgen.c diff --git a/test/wcc/916_arr_float_call_index_run.c b/test/wcc/916_arr_float_call_index_run.c new file mode 100644 index 00000000..e2ce866f --- /dev/null +++ b/test/wcc/916_arr_float_call_index_run.c @@ -0,0 +1,239 @@ +/* + * 916_arr_float_call_index_run — runtime + byte-id net for #125: in + * the `arr[i] = v` ASSIGN path, when the element type is float and the + * INDEX sub-expression clobbers X0 (e.g. a fn-call index), the value + * was LOST pre-fix because both stages PUSHed AX (junk for floats) + + * never spilled X0 across the idx/base eval, then re-emitted MOVSS/ + * MOVSD X0, (BX) using the already-clobbered X0. + * + * The bug was load-bearing only after #122 routed the value through + * X0 (pre-#122 it sat in AX and the existing PUSH AX accidentally + * protected it). Broken byte-identically between cstage and wwstage, + * so pre-existing + non-regression but exposed by #122. + * + * Fix mirrors the *p = v float deref-store at cmd/w6c/cgen.c:4187 + * (the only other arr[i]= /-style float store): for float elements, + * substitute the PUSHQ AX / POPQ AX pair around the idx/base eval + * with SUBQ $8,SP + MOVSS/MOVSD X0,(SP) ... MOVSS/MOVSD (SP),X0 + + * ADDQ $8,SP. Non-float keeps PUSHQ AX (the original) so the str/ + * slice 3-word pop order at the end of the branch is undisturbed. + * + * The fix changes asm shape for ALL float arr[i]= stores (its own + * scoped fold, byte-identical between stages); covered by the 990- + * 997 byte-id gates + this test's standalone w6c vs w6c_ww cmp. + * + * Rows cover: + * - f64_call_index: a[geti(1.0)] = 1.5f64 — the canonical repro, + * geti is a fn that adds 1 and returns i32 (its body clobbers X0). + * Pre-fix exit=2 (a[2] held geti's last X0=2.0 → cast i32=2); + * post-fix exit=1 (a[2] = 1.5 → cast i32 = 1). + * - f32_call_index: same shape, f32 element — pre-fix even worse + * because CVTSD2SS at #104 only touched X0; AX store would be + * garbage. Post-fix exit=1 same as f64. + * - f64_lit_index: a[2] = 1.5f64 with a LITERAL index — pre-fix + * this already worked (cgexpr of an int literal doesn't clobber + * X0). Regression guard. + * - f64_localvar_index: a[k] = 1.5f64 with k = 2 (local var) — + * pre-fix also worked (MOVQ off(BP), AX doesn't clobber X0). + * Regression guard. + * - f64_arith_index: a[k + 1] = 1.5f64 — integer arith on locals + * doesn't clobber X0 either; pre-fix worked. Regression guard. + * + * Each row carries (a) cstage `ww build` + run asserting exit code + * and (b) w6c vs w6c_ww `.s` cmp (rule-10 byte-id). + */ +#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_exit; }; + +static const struct row rows[] = { + { "f64_call_index", + "package main;\n" + "export fn geti(x: f64) i32 = {\n" + " let y: f64 = x + 1.0;\n" + " return (y: i32);\n" + "};\n" + "export fn main() i32 = {\n" + " let a: [4]f64 = [99.0, 99.0, 99.0, 99.0];\n" + " a[geti(1.0)] = 1.5f64;\n" + " let v: i32 = (a[2]: i32);\n" + " return v;\n" + "};\n", 1 }, + /* Body of getj uses f64 arith → clobbers X0 via the materialise- + * literal-in-X0 sequence and the ADDSD. The call's argument is + * int (not float), so the call-arg-push goes through the integer + * convention and avoids a pre-existing cs/ww f32-arg-push MOVSD- + * vs-MOVSS divergence in pre-call arg materialisation (sibling + * bug, filed separately). Pre-fix the post-call X0 (set to the + * residue of 1.0+1.0 = 2.0) would overwrite the 1.5f32 in X0, + * yielding garbage. */ + { "f32_call_index", + "package main;\n" + "export fn getj(seed: i32) i32 = {\n" + " let y: f64 = (seed: f64) + 1.0;\n" + " return (y: i32);\n" + "};\n" + "export fn main() i32 = {\n" + " let a: [4]f32 = [99.0f32, 99.0f32, 99.0f32, 99.0f32];\n" + " a[getj(1)] = 1.5f32;\n" + " let v: i32 = (a[2]: i32);\n" + " return v;\n" + "};\n", 1 }, + { "f64_lit_index", + "package main;\n" + "export fn main() i32 = {\n" + " let a: [4]f64 = [99.0, 99.0, 99.0, 99.0];\n" + " a[2] = 1.5f64;\n" + " let v: i32 = (a[2]: i32);\n" + " return v;\n" + "};\n", 1 }, + { "f64_localvar_index", + "package main;\n" + "export fn main() i32 = {\n" + " let a: [4]f64 = [99.0, 99.0, 99.0, 99.0];\n" + " let k: i32 = 2;\n" + " a[k] = 1.5f64;\n" + " let v: i32 = (a[2]: i32);\n" + " return v;\n" + "};\n", 1 }, + { "f64_arith_index", + "package main;\n" + "export fn main() i32 = {\n" + " let a: [4]f64 = [99.0, 99.0, 99.0, 99.0];\n" + " let k: i32 = 1;\n" + " a[k + 1] = 1.5f64;\n" + " let v: i32 = (a[2]: i32);\n" + " return v;\n" + "};\n", 1 }, + { NULL, NULL, 0 } +}; + +static int +slurp_eq(const char *a, const char *b) +{ + FILE *fa = fopen(a, "rb"); + FILE *fb = fopen(b, "rb"); + if (!fa || !fb) { if (fa) fclose(fa); if (fb) fclose(fb); return -1; } + int rc = 0; + for (;;) { + int ca = fgetc(fa); + int cb = fgetc(fb); + if (ca != cb) { rc = -1; break; } + if (ca == EOF) break; + } + fclose(fa); fclose(fb); + return rc; +} + +int +main(void) +{ + const char *bin = getenv("BIN"); + if (!bin) bin = "out/bin"; + char absbin[1024]; + if (bin[0] != '/') { + char cwd[1024]; + if (getcwd(cwd, sizeof cwd) == NULL) return 1; + snprintf(absbin, sizeof absbin, "%s/%s", cwd, bin); + bin = absbin; + } + + char w6c[1100], w6c_ww[1100]; + snprintf(w6c, sizeof w6c, "%s/w6c", bin); + snprintf(w6c_ww, sizeof w6c_ww, "%s/w6c_ww", bin); + if (access(w6c_ww, X_OK) != 0) { + fprintf(stderr, "arrfcidx: w6c_ww missing — cannot run " + "the cs==ww byte-id gate (the whole point of this test)\n"); + return 1; + } + + int n = 0, fail = 0; + for (int i = 0; rows[i].src; i++, n++) { + char src[64]; + snprintf(src, sizeof src, "/tmp/wwafc_%d_%d.ww", getpid(), i); + FILE *f = fopen(src, "wb"); + if (f == NULL) { fail++; continue; } + fputs(rows[i].src, f); + fclose(f); + + char tmpdir[64]; + snprintf(tmpdir, sizeof tmpdir, "/tmp/wwafc_%d_d_%d", + getpid(), i); + mkdir(tmpdir, 0755); + + char cmd[2048]; + snprintf(cmd, sizeof cmd, "cd %s && %s/ww build %s", + tmpdir, bin, src); + if (runwait(cmd) != 0) { + fprintf(stderr, "row[%s]: cstage build failed\n", + rows[i].label); + fail++; + unlink(src); rmdir(tmpdir); + continue; + } + + char outbin[128]; + const char *base = strrchr(src, '/'); + base = base ? base + 1 : src; + snprintf(outbin, sizeof outbin, "%s/%s", tmpdir, base); + char *dot = strrchr(outbin, '.'); + if (dot && strcmp(dot, ".ww") == 0) *dot = '\0'; + + int got = runwait(outbin); + if (got != rows[i].want_exit) { + fprintf(stderr, "row[%s]: cstage exit %d, want %d\n", + rows[i].label, got, rows[i].want_exit); + fail++; + } + unlink(outbin); rmdir(tmpdir); + + char cs_s[64], ws_s[64]; + snprintf(cs_s, sizeof cs_s, "/tmp/wwafc_%d_%d_cs.s", + getpid(), i); + snprintf(ws_s, sizeof ws_s, "/tmp/wwafc_%d_%d_ww.s", + getpid(), i); + + snprintf(cmd, sizeof cmd, "%s -o %s %s 2>/dev/null", + w6c, cs_s, src); + if (runwait(cmd) != 0) { + fprintf(stderr, "row[%s]: w6c failed\n", rows[i].label); + fail++; unlink(src); continue; + } + snprintf(cmd, sizeof cmd, "%s -o %s %s 2>/dev/null", + w6c_ww, ws_s, src); + if (runwait(cmd) != 0) { + fprintf(stderr, "row[%s]: w6c_ww failed\n", + rows[i].label); + fail++; unlink(src); unlink(cs_s); continue; + } + if (slurp_eq(cs_s, ws_s) != 0) { + fprintf(stderr, + "row[%s]: cstage/wwstage .s DIFFER (rule-10 " + "byte-id violation)\n", rows[i].label); + fail++; + } + unlink(src); unlink(cs_s); unlink(ws_s); + } + + if (fail) { + fprintf(stderr, "%d/%d arr-float-call-idx tests failed\n", fail, n); + return 1; + } + printf("arrfcidx: %d/%d ok (cstage run + cs==ww byte-id)\n", + n, n); + return 0; +}