cstage+selfhost+test: sign-aware codegen for signed int div/mod (#16)
Shared miscompile in both stages — not a divergence. Bootstrap byte-id
passed throughout because both stages emitted the same wrong asm. Both
the C cgen (cmd/w6c/cgen.c TK_SLASH/TK_PERCENT) and the ww cgen
(selfhost/cmd/wcc/cgenexpr.ww) prepped IDIVQ with `MOVQ $0, DX`, which
is the unsigned 128-bit dividend shape. For a negative RAX, the CPU
then divides 2^64 + (-RAX) by the divisor — unsigned wraparound, not
signed division. Surfaced via lib/time/add() needing the verbatim Hare
signed-%-normalisation in ref/hare/time/arithm.ha.
Fix: emit CQO (sign-extend RAX into RDX:RAX, REX.W 99) on the signed
arm; keep MOVQ $0, DX on the unsigned arm where the DIVQ-vs-IDIVQ
dispatch was already correct. Since both stages always emit 64-bit
IDIVQ regardless of source width, a single CQO suffices for
i64/i32/i16/i8 — the dividend already lives in RAX sign-extended. No
CDQ/CWTL/CBTW needed.
Symmetric stages (rule 10): both stages were broken identically; both
get the same surgical fix. Adds A_CQO to each assembler's opcode set:
cstage in cmd/w6c/6.out.h + cmd/w6c/txt.c + cmd/w6a/{parse,asm}.c;
wwstage in selfhost/cmd/w6a/{types,parse,asm}.ww.
Class B (shared miscompile) — new in the session's polarity catalog.
Bootstrap byte-id is useless for catching it; semantic 9xx runtime
tests are the right shape. test/wcc/978_intdiv_signed.c covers 27 rows
× 2 drivers = 54 fixtures across {i8,i16,i32,i64,u8,u16,u32,u64} ×
{/, %} with width-boundary minima (INT8_MIN, INT16_MIN, INT32_MIN,
INT64_MIN/2) and high-bit-set unsigned anchors. INT64_MIN is spelled
(-INT64_MAX) - 1 per task #17 (wwstage NEGQ-over-imm drops digits on
-9223372036854775808i64); that literal-cgen bug is unrelated to this
fix.
Two known compound-assign workarounds at cmd/w6c/cgen.c:3765
(TK_SLASHEQ IDENT-local) and :3549 (TK_SLASHEQ/TK_PERCENTEQ
deref-compound) remain in tree; both depend on the assembler having
CQO, so they revert in a follow-up commit citing this one.
This commit is contained in:
@@ -15,6 +15,7 @@
|
||||
* IMULQ reg, reg — 0F AF /r (REX.W)
|
||||
* IDIVQ reg — F7 /7 (REX.W)
|
||||
* DIVQ reg — F7 /6 (REX.W) (unsigned)
|
||||
* CQO — REX.W 99 (sign-extend RAX→RDX:RAX)
|
||||
* NEGQ/NOTQ reg — F7 /3, F7 /2 (REX.W)
|
||||
* SHLQ/SHRQ CL, reg — D3 /4, D3 /5 (REX.W)
|
||||
* CMPQ reg, reg — 39 /r (REX.W)
|
||||
@@ -396,6 +397,10 @@ a_encode(Asm *a)
|
||||
/* unsigned divide; shares the F7 group with IDIVQ but
|
||||
* uses /6 instead of /7. */
|
||||
encode_unary(a, 0xF7, 6, p->to.type); break;
|
||||
case A_CQO:
|
||||
/* REX.W 99 — sign-extend RAX into RDX:RAX. */
|
||||
a_emit_byte(a, 0x48); a_emit_byte(a, 0x99);
|
||||
break;
|
||||
|
||||
case A_MOVQ:
|
||||
if (p->from.type == D_CONST && p->to.type >= D_AX && p->to.type <= D_R15) {
|
||||
|
||||
@@ -103,7 +103,7 @@ opcode_lookup(const char *m)
|
||||
{ "CVTSS2SD", A_CVTSS2SD },
|
||||
{ "ADDQ", A_ADDQ }, { "SUBQ", A_SUBQ },
|
||||
{ "IMULQ",A_IMULQ},{ "IDIVQ",A_IDIVQ},
|
||||
{ "DIVQ", A_DIVQ },
|
||||
{ "DIVQ", A_DIVQ },{ "CQO", A_CQO },
|
||||
{ "NEGQ", A_NEGQ },{ "NOTQ", A_NOTQ },
|
||||
{ "ANDQ", A_ANDQ },{ "ORQ", A_ORQ },
|
||||
{ "XORQ", A_XORQ },
|
||||
|
||||
@@ -83,6 +83,9 @@ enum {
|
||||
A_IMULQ,
|
||||
A_IDIVQ,
|
||||
A_DIVQ, /* unsigned 64-bit divide; sibling of IDIVQ */
|
||||
A_CQO, /* sign-extend RAX into RDX:RAX; the signed-division
|
||||
* prep that pairs with IDIVQ (DIVQ pairs with a
|
||||
* MOVQ $0, DX zero-fill). */
|
||||
A_NEGQ,
|
||||
A_NOTQ,
|
||||
A_ANDQ,
|
||||
|
||||
@@ -2166,17 +2166,26 @@ cgexpr(Cg *c, Node *n, Local *locals)
|
||||
/* Use DIV (unsigned) when either operand is an unsigned
|
||||
* integer type — IDIV would sign-extend a u64 with high
|
||||
* bit set into a negative i64 and produce wrong results
|
||||
* (see strconv.u64tos with v = 1 << 63). */
|
||||
* (see strconv.u64tos with v = 1 << 63). Signed IDIV
|
||||
* needs CQO to sign-extend RAX into RDX:RAX; zeroing
|
||||
* DX would treat a negative dividend as a huge unsigned
|
||||
* 128-bit value. */
|
||||
int unsignd = (n->lhs && type_isunsigned(n->lhs->type))
|
||||
|| (n->rhs && type_isunsigned(n->rhs->type));
|
||||
ins2(c, A_MOVQ, aimm(0), areg(D_DX));
|
||||
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_BX));
|
||||
break;
|
||||
}
|
||||
case TK_PERCENT: {
|
||||
int unsignd = (n->lhs && type_isunsigned(n->lhs->type))
|
||||
|| (n->rhs && type_isunsigned(n->rhs->type));
|
||||
ins2(c, A_MOVQ, aimm(0), areg(D_DX));
|
||||
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_BX));
|
||||
ins2(c, A_MOVQ, areg(D_DX), areg(D_AX));
|
||||
break;
|
||||
|
||||
@@ -61,6 +61,7 @@ anames(int op)
|
||||
case A_IMULQ: return "IMULQ";
|
||||
case A_IDIVQ: return "IDIVQ";
|
||||
case A_DIVQ: return "DIVQ";
|
||||
case A_CQO: return "CQO";
|
||||
case A_NEGQ: return "NEGQ";
|
||||
case A_NOTQ: return "NOTQ";
|
||||
case A_ANDQ: return "ANDQ";
|
||||
@@ -164,6 +165,9 @@ txt_emit(FILE *f, Prog *head)
|
||||
case A_SYSCALL:
|
||||
fputs("\tSYSCALL\n", f);
|
||||
break;
|
||||
case A_CQO:
|
||||
fputs("\tCQO\n", f);
|
||||
break;
|
||||
case A_NEGQ:
|
||||
case A_NOTQ:
|
||||
case A_PUSHQ:
|
||||
|
||||
Reference in New Issue
Block a user