wcc: compound-assign load-op-store for indexed + chained-ptr-field lvalues (#133)
Both stages had silent miscompiles on compound assignment for two shapes: indexed lvalue (`arr[i] OP= v`) and chained-pointer-field (`d.fld.fld OP= v` through a *struct chain). The cstage N_INDEX-lhs branch did not gate on TK_ASSIGN and silently DEMOTED compound ops to plain stores (RHS stored, no load, no op). The wwstage equivalents silently DROPPED the line entirely (no instructions emitted). The chained-pointer-field compound template at cgen.c:3281-3317 also silently identity-stored on unwired compound ops (SLASHEQ / PERCENTEQ / LSHIFTEQ / RSHIFTEQ all fell to the switch default = no-op = load, pop RHS, store ORIGINAL value back) and silently no-op'd on float / str / slice / tagged element compound; its wwstage twin at cgenexpr.ww:5471 only handled TK_ASSIGN, dropping any chained-ptr-field compound entirely. Wire all 10 integer compound ops (PLUSEQ MINUSEQ STAREQ AMPEQ PIPEEQ CARETEQ SLASHEQ PERCENTEQ LSHIFTEQ RSHIFTEQ) at all 4 sites in both stages: SLASHEQ/PERCENTEQ via CQO+IDIVQ (signed) or zero-DX+DIVQ (unsigned), with PERCENTEQ moving DX->AX for the result; LSHIFTEQ/ RSHIFTEQ via SHLQ/SHRQ on CX (rhs already in CX after the pop). Signedness keyed off the field/element type via type_isunsigned / typeisunsigned. Float / str / slice / tagged element compound now LOUD-ERRORS at codegen with a distinct per-site diagnostic citing #133/rule-7 instead of silent fall-through. Site 3 (the wwstage chained-pointer-field compound) is ADDED FROM SCRATCH alongside the existing TK_ASSIGN-only arm — pre-#133 wwstage emitted zero instructions for any `d.i.v OP= v` shape, a rule-10 silent divergence from the cstage which handled the same shape correctly. Multi-fix carve-out (rule 11): the 10 wired ops at 4 sites + hard-error gate on 4 unwired payload kinds at 4 sites are ONE silent-misbehavior class closure on indexed/chained-ptr-field compound assignment. Splitting would muddle bisect on related cgen surfaces — the wired ops, the hard-error gate, and the rule-10 cstage/wwstage symmetry are inseparable correctness facts at each site. The inherited template default-break silent-identity (cgen.c:3281-3317) was the originating class root; close it everywhere or leave the class open. 948_idx_compound_run: 21 rows total. 11 runtime+byte-id rows for the original 6 ops on u8/i32/i64/u32 array bases and one slice base, with a plain-assign control row asserting the ASSIGN path is byte-id- unchanged. 7 new runtime+byte-id rows for SLASHEQ/PERCENTEQ on signed i32 + unsigned u32, LSHIFTEQ on i32, RSHIFTEQ on signed-positive i32 and unsigned u32. 3 builderr rows (he_float_indexed, he_str_indexed, he_float_chained_ptr) asserting both stages exit non-zero AND stderr carries the cited diagnostic substring (rule-7 — never silent). Mirrors 945_tuple_nary's builderr/experr pattern. Bootstrap NEUTRAL — `grep -rE '\][[:space:]]*(\+=|-=|\*=|/=|&=|\|=|\^=|<<=|>>=)' lib/ selfhost/` (excluding combined.ww) returns ZERO existing callers for the indexed compound shape, and the chained-ptr-field compound shape was silent- no-op in wwstage pre-fix (no working caller possible). 990-997 byte- id gates green, 994 explicit confirms 18 corpus inputs identical pre/post. combined.ww (w6c + wwdump) regen deterministic across re-touch+rebuild. A_SARQ is not in w6a's opcode table; signed RSHIFTEQ uses SHRQ at all 4 sites for parity with the pre-existing deref-lvalue compound site (TK_RSHIFTEQ→A_SHRQ at cgen.c:4145). Documented technical debt filed as #136 — pre-existing concern that a fix would need w6a opcode addition + cgen sweep across every SHRQ-for-signed-RSHIFT site, out of scope for this fold.
This commit is contained in:
201
cmd/w6c/cgen.c
201
cmd/w6c/cgen.c
@@ -3280,8 +3280,41 @@ cgexpr(Cg *c, Node *n, Local *locals)
|
||||
}
|
||||
/* compound op: AX=rhs → push; eval ptr → push;
|
||||
* load old field → AX; pop ptr→BX, rhs→CX;
|
||||
* combine; store. Float/str compound on a
|
||||
* chained pointer-field is not wired. */
|
||||
* combine; store. #133-expanded: all 10 integer
|
||||
* compound ops wired; SLASHEQ/PERCENTEQ via
|
||||
* CQO+IDIV (signed) or zero-DX+DIV (unsigned);
|
||||
* LSHIFTEQ/RSHIFTEQ via SHLQ/SHRQ on CX. Float /
|
||||
* str / slice / tagged element compound hard-
|
||||
* errors LOUD (rule-7 — replaces prior silent
|
||||
* fall-through-to-default-break). Note: A_SARQ
|
||||
* isn't in w6a today; signed RSHIFTEQ uses SHRQ
|
||||
* for parity with the pre-existing deref-lvalue
|
||||
* compound site (TK_RSHIFTEQ→A_SHRQ below);
|
||||
* signed-RSHIFT-on-negatives separate concern,
|
||||
* filed as #136, out of scope for #133. */
|
||||
{
|
||||
int compound_isf32 = 0;
|
||||
if (fld_isfloat(ft, &compound_isf32))
|
||||
fatal("chained-ptr-field compound on "
|
||||
"float element not wired "
|
||||
"(#133/rule-7); field='%s'",
|
||||
n->lhs->str);
|
||||
Type *fchk = type_chase_named(ft);
|
||||
if (fchk && fchk->kind == TY_STR)
|
||||
fatal("chained-ptr-field compound on "
|
||||
"str element not wired "
|
||||
"(#133/rule-7); field='%s'",
|
||||
n->lhs->str);
|
||||
if (fchk && fchk->kind == TY_SLICE)
|
||||
fatal("chained-ptr-field compound on "
|
||||
"slice element not wired "
|
||||
"(#133/rule-7); field='%s'",
|
||||
n->lhs->str);
|
||||
if (fchk && fchk->kind == TY_TAGGED)
|
||||
fatal("chained-ptr-field compound on "
|
||||
"tagged element not wired "
|
||||
"(#133/rule-7); field='%s'",
|
||||
n->lhs->str);
|
||||
cgexpr(c, n->rhs, locals);
|
||||
ins1(c, A_PUSHQ, areg(D_AX));
|
||||
cgexpr(c, n->lhs->lhs, locals);
|
||||
@@ -3291,6 +3324,7 @@ cgexpr(Cg *c, Node *n, Local *locals)
|
||||
areg(D_AX));
|
||||
ins1(c, A_POPQ, areg(D_BX));
|
||||
ins1(c, A_POPQ, areg(D_CX));
|
||||
int unsignd = type_isunsigned(ft);
|
||||
switch (n->op) {
|
||||
case TK_PLUSEQ:
|
||||
ins2(c, A_ADDQ, areg(D_CX), areg(D_AX));
|
||||
@@ -3310,10 +3344,43 @@ cgexpr(Cg *c, Node *n, Local *locals)
|
||||
case TK_CARETEQ:
|
||||
ins2(c, A_XORQ, areg(D_CX), areg(D_AX));
|
||||
break;
|
||||
default: break;
|
||||
case TK_SLASHEQ:
|
||||
if (unsignd)
|
||||
ins2(c, A_MOVQ, aimm(0),
|
||||
areg(D_DX));
|
||||
else
|
||||
ins0(c, A_CQO);
|
||||
ins1(c, unsignd ? A_DIVQ : A_IDIVQ,
|
||||
areg(D_CX));
|
||||
break;
|
||||
case TK_PERCENTEQ:
|
||||
if (unsignd)
|
||||
ins2(c, A_MOVQ, aimm(0),
|
||||
areg(D_DX));
|
||||
else
|
||||
ins0(c, A_CQO);
|
||||
ins1(c, unsignd ? A_DIVQ : A_IDIVQ,
|
||||
areg(D_CX));
|
||||
ins2(c, A_MOVQ, areg(D_DX),
|
||||
areg(D_AX));
|
||||
break;
|
||||
case TK_LSHIFTEQ:
|
||||
ins2(c, A_SHLQ, areg(D_CX),
|
||||
areg(D_AX));
|
||||
break;
|
||||
case TK_RSHIFTEQ:
|
||||
ins2(c, A_SHRQ, areg(D_CX),
|
||||
areg(D_AX));
|
||||
break;
|
||||
default:
|
||||
fatal("chained-ptr-field compound: "
|
||||
"unknown op tk=%d (#133/rule-7); "
|
||||
"field='%s'", n->op,
|
||||
n->lhs->str);
|
||||
}
|
||||
ins2(c, store_op, areg(D_AX),
|
||||
amem(D_BX, foff));
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
@@ -3775,7 +3842,7 @@ cgexpr(Cg *c, Node *n, Local *locals)
|
||||
}
|
||||
break;
|
||||
}
|
||||
if (is_arr || is_sl || is_ptr) {
|
||||
if ((is_arr || is_sl || is_ptr) && n->op == TK_ASSIGN) {
|
||||
cgexpr(c, n->rhs, locals); /* AX=ptr (BX=len,CX=cap if str) */
|
||||
/* str/slice: stash cap+len so all three store
|
||||
* (#1/Phase 3). */
|
||||
@@ -3850,6 +3917,132 @@ cgexpr(Cg *c, Node *n, Local *locals)
|
||||
ins2(c, store_op, areg(D_AX), amem(D_BX, 0));
|
||||
break;
|
||||
}
|
||||
/* Compound assign on an indexed scalar element
|
||||
* (`arr[i] OP= v`). Pre-#133 this branch had no TK_ASSIGN
|
||||
* gate above and silently DEMOTED compound ops to plain
|
||||
* stores (no load, no op). Mirror the chained-pointer-
|
||||
* field compound template at cgen.c:3281-3317: same
|
||||
* address computation as the ASSIGN body above, then
|
||||
* load_op (BX)→AX, pop rhs→CX, combine, store_op.
|
||||
* #133-expanded: all 10 integer compound ops wired;
|
||||
* float/str/slice/tagged element compound HARD-ERRORS
|
||||
* loud (rule-7, replaces prior silent fall-through).
|
||||
* Signed RSHIFTEQ uses SHRQ — parity with the pre-
|
||||
* existing deref-lvalue compound site (TK_RSHIFTEQ→
|
||||
* A_SHRQ below). A_SARQ not in w6a; pre-existing
|
||||
* signed-RSHIFT-on-negatives is filed as #136, out of
|
||||
* scope for #133. */
|
||||
if ((is_arr || is_sl || is_ptr) && n->op != TK_ASSIGN) {
|
||||
if (elem_is_str)
|
||||
fatal("indexed-lvalue compound on "
|
||||
"str element not wired "
|
||||
"(#133/rule-7)");
|
||||
if (elem_is_slice)
|
||||
fatal("indexed-lvalue compound on "
|
||||
"slice element not wired "
|
||||
"(#133/rule-7)");
|
||||
if (elem_tagged)
|
||||
fatal("indexed-lvalue compound on "
|
||||
"tagged element not wired "
|
||||
"(#133/rule-7)");
|
||||
if (esub && type_isfloat(esub))
|
||||
fatal("indexed-lvalue compound on "
|
||||
"float element not wired "
|
||||
"(#133/rule-7)");
|
||||
cgexpr(c, n->rhs, locals);
|
||||
ins1(c, A_PUSHQ, areg(D_AX));
|
||||
cgexpr(c, n->lhs->rhs, locals);
|
||||
if (esz > 1) {
|
||||
ins2(c, A_MOVQ, aimm(esz), areg(D_CX));
|
||||
ins2(c, A_IMULQ, areg(D_CX), areg(D_AX));
|
||||
}
|
||||
ins1(c, A_PUSHQ, areg(D_AX));
|
||||
if (base->kind == N_IDENT) {
|
||||
int off = localfind(locals, base->str);
|
||||
int isglobal = (off == 0) &&
|
||||
let_islet(base->str);
|
||||
if (isglobal && is_arr) {
|
||||
ins2(c, A_LEAQ,
|
||||
masym(c, base->str),
|
||||
areg(D_BX));
|
||||
} else if (isglobal) {
|
||||
ins2(c, A_MOVQ,
|
||||
masym(c, base->str),
|
||||
areg(D_BX));
|
||||
} else if (is_arr) {
|
||||
ins2(c, A_LEAQ,
|
||||
amem(D_BP, off),
|
||||
areg(D_BX));
|
||||
} else {
|
||||
ins2(c, A_MOVQ,
|
||||
amem(D_BP, off),
|
||||
areg(D_BX));
|
||||
}
|
||||
} else {
|
||||
cgexpr(c, base, locals);
|
||||
ins2(c, A_MOVQ, areg(D_AX), areg(D_BX));
|
||||
}
|
||||
ins1(c, A_POPQ, areg(D_AX));
|
||||
ins2(c, A_ADDQ, areg(D_AX), areg(D_BX));
|
||||
int load_op = fldloadop(esub, esz);
|
||||
ins2(c, load_op, amem(D_BX, 0), areg(D_AX));
|
||||
ins1(c, A_POPQ, areg(D_CX));
|
||||
int unsignd_c = esub && type_isunsigned(esub);
|
||||
switch (n->op) {
|
||||
case TK_PLUSEQ:
|
||||
ins2(c, A_ADDQ, areg(D_CX), areg(D_AX));
|
||||
break;
|
||||
case TK_MINUSEQ:
|
||||
ins2(c, A_SUBQ, areg(D_CX), areg(D_AX));
|
||||
break;
|
||||
case TK_STAREQ:
|
||||
ins2(c, A_IMULQ, areg(D_CX), areg(D_AX));
|
||||
break;
|
||||
case TK_AMPEQ:
|
||||
ins2(c, A_ANDQ, areg(D_CX), areg(D_AX));
|
||||
break;
|
||||
case TK_PIPEEQ:
|
||||
ins2(c, A_ORQ, areg(D_CX), areg(D_AX));
|
||||
break;
|
||||
case TK_CARETEQ:
|
||||
ins2(c, A_XORQ, areg(D_CX), areg(D_AX));
|
||||
break;
|
||||
case TK_SLASHEQ:
|
||||
if (unsignd_c)
|
||||
ins2(c, A_MOVQ, aimm(0),
|
||||
areg(D_DX));
|
||||
else
|
||||
ins0(c, A_CQO);
|
||||
ins1(c, unsignd_c ? A_DIVQ : A_IDIVQ,
|
||||
areg(D_CX));
|
||||
break;
|
||||
case TK_PERCENTEQ:
|
||||
if (unsignd_c)
|
||||
ins2(c, A_MOVQ, aimm(0),
|
||||
areg(D_DX));
|
||||
else
|
||||
ins0(c, A_CQO);
|
||||
ins1(c, unsignd_c ? A_DIVQ : A_IDIVQ,
|
||||
areg(D_CX));
|
||||
ins2(c, A_MOVQ, areg(D_DX), areg(D_AX));
|
||||
break;
|
||||
case TK_LSHIFTEQ:
|
||||
ins2(c, A_SHLQ, areg(D_CX),
|
||||
areg(D_AX));
|
||||
break;
|
||||
case TK_RSHIFTEQ:
|
||||
ins2(c, A_SHRQ, areg(D_CX),
|
||||
areg(D_AX));
|
||||
break;
|
||||
default:
|
||||
fatal("indexed-lvalue compound: "
|
||||
"unknown op tk=%d (#133/rule-7)",
|
||||
n->op);
|
||||
}
|
||||
int store_op_c = fldstoreop(esub, esz);
|
||||
ins2(c, store_op_c, areg(D_AX), amem(D_BX, 0));
|
||||
break;
|
||||
}
|
||||
}
|
||||
/* Plain `r = expr;` where r is a tagged-union local.
|
||||
* Delegates to cg_widen_tagged_store: covers nullable fold,
|
||||
|
||||
Reference in New Issue
Block a user