From 7d7ed964b0867e965cd353f5125501ff970df702 Mon Sep 17 00:00:00 2001 From: Hojun-Cho Date: Tue, 26 May 2026 14:46:01 +0900 Subject: [PATCH] wcc: f32 array-element store from X0 (#122) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Float array-element stores (array-literal init, [v...] repeat-fill, and arr[i]=v) now route from X0 via MOVSS/MOVSD in both stages; the AX path stored the raw double low-bits, garbage for f32 (f64 worked by accident). A clobbering call-index (a[geti()]=v) loses the X0 value — deferred to #125. --- cmd/w6c/cgen.c | 27 ++++++++++++++ selfhost/cmd/w6c/main.combined.ww | 42 ++++++++++++++++++++-- selfhost/cmd/wcc/cgenexpr.ww | 18 ++++++++++ selfhost/cmd/wcc/cgenstmt.ww | 24 +++++++++++-- selfhost/cmd/wwdump/main.combined.ww | 42 ++++++++++++++++++++-- test/wcc/946_floatarr_run.c | 54 +++++++++++++++++++++------- 6 files changed, 188 insertions(+), 19 deletions(-) diff --git a/cmd/w6c/cgen.c b/cmd/w6c/cgen.c index 34cfee68..a7c9a1a0 100644 --- a/cmd/w6c/cgen.c +++ b/cmd/w6c/cgen.c @@ -3815,6 +3815,22 @@ cgexpr(Cg *c, Node *n, Local *locals) ins2(c, A_MOVQ, areg(D_CX), amem(D_BX, 16)); break; } + /* float element → store FROM X0 (MOVSS/MOVSD): cgexpr + * leaves a float value in X0, and for f32 the #104 + * 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. */ + if (type_isfloat(esub)) { + int mov = type_isf32(esub) ? A_MOVSS : A_MOVSD; + ins2(c, mov, areg(D_X0), amem(D_BX, 0)); + break; + } int store_op = fldstoreop(esub, esz); ins2(c, store_op, areg(D_AX), amem(D_BX, 0)); break; @@ -6867,6 +6883,11 @@ cgstmt(Cg *c, Node *n, Local **locals, int *frame) Type *esub = lu->sub; int esz = esub ? (int)esub->size : 1; int is_str_el = type_isstr(esub); + /* float element → store FROM X0; the AX path stores + * raw double low-bits, garbage for f32 (#122, twin of + * the arr[i]= store fix and the cgen.c:6423 read). */ + int is_float_el = type_isfloat(esub); + int fmov = type_isf32(esub) ? A_MOVSS : A_MOVSD; int op = A_MOVQ; if (!is_str_el) { if (esz == 1) op = A_MOVB; @@ -6893,6 +6914,9 @@ cgstmt(Cg *c, Node *n, Local **locals, int *frame) amem(D_BP, base)); ins2(c, A_MOVQ, areg(D_BX), amem(D_BP, base + 8)); + } else if (is_float_el) { + ins2(c, fmov, areg(D_X0), + amem(D_BP, base)); } else { ins2(c, op, areg(D_AX), amem(D_BP, base)); @@ -6910,6 +6934,9 @@ cgstmt(Cg *c, Node *n, Local **locals, int *frame) amem(D_BP, base)); ins2(c, A_MOVQ, areg(D_BX), amem(D_BP, base + 8)); + } else if (is_float_el) { + ins2(c, fmov, areg(D_X0), + amem(D_BP, base)); } else { ins2(c, op, areg(D_AX), amem(D_BP, base)); diff --git a/selfhost/cmd/w6c/main.combined.ww b/selfhost/cmd/w6c/main.combined.ww index f35bf3bf..0e980330 100644 --- a/selfhost/cmd/w6c/main.combined.ww +++ b/selfhost/cmd/w6c/main.combined.ww @@ -18444,6 +18444,24 @@ fn cgassign(c: *cgen, n: *node) void = { emitline("\tMOVQ\tCX, 16(BX)\n"); 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. + // 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 + // arr[i]= float store). + if (isfloattype(c, elemtn)) { + let fmov: str = "MOVSD"; + if (isf32type(c, elemtn)) { fmov = "MOVSS"; }; + emitline("\t"); + emitline(fmov); + emitline("\tX0, (BX)\n"); + return; + }; let isop: str = tnodestoreop(c, elemtn, esz); emitline("\t"); emitline(isop); @@ -21435,6 +21453,14 @@ fn cglet(c: *cgen, n: *node) void = { }; }; let mop: str = tnodestoreop(c, elemn, esz); + // float element → store FROM X0 (MOVSS/MOVSD): cgexpr + // leaves a float in X0 and for f32 the #104 CVTSD2SS + // narrowing only touches X0; the AX store (mop) would + // write the raw double low-bits, garbage for f32 (#122, + // mirrors cstage cgen.c:6889 arr-lit float store). + let isfloatel: bool = isfloattype(c, elemn); + let fmov: str = "MOVSD"; + if (isf32type(c, elemn)) { fmov = "MOVSS"; }; let idx: i32 = 0; let repeat: bool = false; let e: *node = rhs.list; @@ -21457,13 +21483,19 @@ fn cglet(c: *cgen, n: *node) void = { emitline("\tMOVQ\tBX, "); emitoff((off + idx * esz + 8): i64); emitline("(BP)\n"); + } else { if (isfloatel) { + emitline("\t"); + emitline(fmov); + emitline("\tX0, "); + emitoff((off + idx * esz): i64); + emitline("(BP)\n"); } else { emitline("\t"); emitline(mop); emitline("\tAX, "); emitoff((off + idx * esz): i64); emitline("(BP)\n"); - }; + }; }; idx += 1; e = e.next; }; @@ -21489,13 +21521,19 @@ fn cglet(c: *cgen, n: *node) void = { emitline("\tMOVQ\tBX, "); emitoff((off + idx * esz + 8): i64); emitline("(BP)\n"); + } else { if (isfloatel) { + emitline("\t"); + emitline(fmov); + emitline("\tX0, "); + emitoff((off + idx * esz): i64); + emitline("(BP)\n"); } else { emitline("\t"); emitline(mop); emitline("\tAX, "); emitoff((off + idx * esz): i64); emitline("(BP)\n"); - }; + }; }; idx += 1; }; }; diff --git a/selfhost/cmd/wcc/cgenexpr.ww b/selfhost/cmd/wcc/cgenexpr.ww index e7828685..448aecf7 100644 --- a/selfhost/cmd/wcc/cgenexpr.ww +++ b/selfhost/cmd/wcc/cgenexpr.ww @@ -4206,6 +4206,24 @@ fn cgassign(c: *cgen, n: *node) void = { emitline("\tMOVQ\tCX, 16(BX)\n"); 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. + // 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 + // arr[i]= float store). + if (isfloattype(c, elemtn)) { + let fmov: str = "MOVSD"; + if (isf32type(c, elemtn)) { fmov = "MOVSS"; }; + emitline("\t"); + emitline(fmov); + emitline("\tX0, (BX)\n"); + return; + }; let isop: str = tnodestoreop(c, elemtn, esz); emitline("\t"); emitline(isop); diff --git a/selfhost/cmd/wcc/cgenstmt.ww b/selfhost/cmd/wcc/cgenstmt.ww index 69ffebe5..82328af7 100644 --- a/selfhost/cmd/wcc/cgenstmt.ww +++ b/selfhost/cmd/wcc/cgenstmt.ww @@ -962,6 +962,14 @@ fn cglet(c: *cgen, n: *node) void = { }; }; let mop: str = tnodestoreop(c, elemn, esz); + // float element → store FROM X0 (MOVSS/MOVSD): cgexpr + // leaves a float in X0 and for f32 the #104 CVTSD2SS + // narrowing only touches X0; the AX store (mop) would + // write the raw double low-bits, garbage for f32 (#122, + // mirrors cstage cgen.c:6889 arr-lit float store). + let isfloatel: bool = isfloattype(c, elemn); + let fmov: str = "MOVSD"; + if (isf32type(c, elemn)) { fmov = "MOVSS"; }; let idx: i32 = 0; let repeat: bool = false; let e: *node = rhs.list; @@ -984,13 +992,19 @@ fn cglet(c: *cgen, n: *node) void = { emitline("\tMOVQ\tBX, "); emitoff((off + idx * esz + 8): i64); emitline("(BP)\n"); + } else { if (isfloatel) { + emitline("\t"); + emitline(fmov); + emitline("\tX0, "); + emitoff((off + idx * esz): i64); + emitline("(BP)\n"); } else { emitline("\t"); emitline(mop); emitline("\tAX, "); emitoff((off + idx * esz): i64); emitline("(BP)\n"); - }; + }; }; idx += 1; e = e.next; }; @@ -1016,13 +1030,19 @@ fn cglet(c: *cgen, n: *node) void = { emitline("\tMOVQ\tBX, "); emitoff((off + idx * esz + 8): i64); emitline("(BP)\n"); + } else { if (isfloatel) { + emitline("\t"); + emitline(fmov); + emitline("\tX0, "); + emitoff((off + idx * esz): i64); + emitline("(BP)\n"); } else { emitline("\t"); emitline(mop); emitline("\tAX, "); emitoff((off + idx * esz): i64); emitline("(BP)\n"); - }; + }; }; idx += 1; }; }; diff --git a/selfhost/cmd/wwdump/main.combined.ww b/selfhost/cmd/wwdump/main.combined.ww index 8af3f613..7dd27129 100644 --- a/selfhost/cmd/wwdump/main.combined.ww +++ b/selfhost/cmd/wwdump/main.combined.ww @@ -18444,6 +18444,24 @@ fn cgassign(c: *cgen, n: *node) void = { emitline("\tMOVQ\tCX, 16(BX)\n"); 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. + // 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 + // arr[i]= float store). + if (isfloattype(c, elemtn)) { + let fmov: str = "MOVSD"; + if (isf32type(c, elemtn)) { fmov = "MOVSS"; }; + emitline("\t"); + emitline(fmov); + emitline("\tX0, (BX)\n"); + return; + }; let isop: str = tnodestoreop(c, elemtn, esz); emitline("\t"); emitline(isop); @@ -21435,6 +21453,14 @@ fn cglet(c: *cgen, n: *node) void = { }; }; let mop: str = tnodestoreop(c, elemn, esz); + // float element → store FROM X0 (MOVSS/MOVSD): cgexpr + // leaves a float in X0 and for f32 the #104 CVTSD2SS + // narrowing only touches X0; the AX store (mop) would + // write the raw double low-bits, garbage for f32 (#122, + // mirrors cstage cgen.c:6889 arr-lit float store). + let isfloatel: bool = isfloattype(c, elemn); + let fmov: str = "MOVSD"; + if (isf32type(c, elemn)) { fmov = "MOVSS"; }; let idx: i32 = 0; let repeat: bool = false; let e: *node = rhs.list; @@ -21457,13 +21483,19 @@ fn cglet(c: *cgen, n: *node) void = { emitline("\tMOVQ\tBX, "); emitoff((off + idx * esz + 8): i64); emitline("(BP)\n"); + } else { if (isfloatel) { + emitline("\t"); + emitline(fmov); + emitline("\tX0, "); + emitoff((off + idx * esz): i64); + emitline("(BP)\n"); } else { emitline("\t"); emitline(mop); emitline("\tAX, "); emitoff((off + idx * esz): i64); emitline("(BP)\n"); - }; + }; }; idx += 1; e = e.next; }; @@ -21489,13 +21521,19 @@ fn cglet(c: *cgen, n: *node) void = { emitline("\tMOVQ\tBX, "); emitoff((off + idx * esz + 8): i64); emitline("(BP)\n"); + } else { if (isfloatel) { + emitline("\t"); + emitline(fmov); + emitline("\tX0, "); + emitoff((off + idx * esz): i64); + emitline("(BP)\n"); } else { emitline("\t"); emitline(mop); emitline("\tAX, "); emitoff((off + idx * esz): i64); emitline("(BP)\n"); - }; + }; }; idx += 1; }; }; diff --git a/test/wcc/946_floatarr_run.c b/test/wcc/946_floatarr_run.c index 236a19f2..6cee6a96 100644 --- a/test/wcc/946_floatarr_run.c +++ b/test/wcc/946_floatarr_run.c @@ -21,13 +21,16 @@ * * Each row carries (a) a cstage `ww build` + run asserting the exit * code, and (b) a w6c vs w6c_ww `.s` cmp (rule-10 byte-id). A row whose - * want_exit is RUN_SKIP runs only the byte-id leg: the f32 row exercises - * the f32 LOAD + cs==ww but its runtime VALUE is blocked by a SEPARATE, - * pre-existing bug — the f32 array-element STORE writes AX (the raw - * double low-bits) instead of the CVTSD2SS-narrowed X0 single, so every - * f32 array slot reads back 0.0f. That store-side twin is filed - * separately (#119-store); this probe still proves the f32 LOAD shape + - * cs==ww byte-identity. + * want_exit is RUN_SKIP runs only the byte-id leg. + * + * #122 fixes the store-side twin the #119 commit deferred: the f32 + * array-element STORE wrote AX (the raw double low-bits) instead of the + * CVTSD2SS-narrowed X0 single, so every f32 array slot read back garbage. + * Both the array-literal-init store (cgen.c:6889 / cgenstmt:949) and the + * arr[i]= index store (cgen.c:3818 / cgenexpr:4209) now route FROM X0 via + * MOVSS/MOVSD, mirroring the scalar float store. The f32 rows below now + * assert the runtime VALUE (not byte-id only) and add an arr[i]= store + * plus a [v...] repeat-fill init (a distinct cgen arm #122 also fixed). */ #include #include @@ -75,17 +78,42 @@ static const struct row rows[] = { " if (a[2] != 9.0) { return 1; };\n" " return 0;\n" "};\n", 0 }, - /* f32 element load (MOVSS into X0) + add, suffixed literals so - * fold-1 narrows them. BYTE-ID ONLY: the runtime value is blocked by - * the f32 array-element STORE bug (#119-store), so we assert only - * that both stages emit the same (correct-load) asm. */ - { "f32_arith_byteid", + /* f32 array-literal-init store + element load + add: suffixed + * literals so fold-1 narrows them. Pre-#122 the init store wrote + * MOVL AX (raw double low-bits) so the slots read garbage; #122 + * routes the store from X0 via MOVSS, so 1.5 + 2.5 == 4.0. */ + { "f32_arith", "package main;\n" "export fn main() i32 = {\n" " let b: [2]f32 = [1.5f32, 2.5f32];\n" " if ((b[0] + b[1]): f64 != 4.0) { return 1; };\n" " return 0;\n" - "};\n", RUN_SKIP }, + "};\n", 0 }, + /* f32 arr[i]= index store (#122): assign each slot, read back. + * The index-store path popped the value to AX and wrote MOVL (raw + * double low-bits, garbage for f32); #122 stores from X0 via MOVSS. + * The [0.0f32,0.0f32] init also exercises the array-lit store. */ + { "f32_index_store", + "package main;\n" + "export fn main() i32 = {\n" + " let b: [2]f32 = [0.0f32, 0.0f32];\n" + " b[0] = 1.5f32;\n" + " b[1] = 2.5f32;\n" + " if ((b[0] + b[1]): f64 != 4.0) { return 1; };\n" + " return 0;\n" + "};\n", 0 }, + /* f32 [v...] repeat-fill init store (#122): the repeat marker + * fills every slot from the last element's X0 single; pre-#122 the + * fill wrote MOVL AX (raw double low-bits) per slot so each read + * back garbage. Distinct cgen arm from the per-element list store. + * 1.5 * 3 == 4.5 (exact in IEEE). */ + { "f32_repeat_fill", + "package main;\n" + "export fn main() i32 = {\n" + " let c: [3]f32 = [1.5f32...];\n" + " if ((c[0] + c[1] + c[2]): f64 != 4.5) { return 1; };\n" + " return 0;\n" + "};\n", 0 }, { NULL, NULL, 0 } };