diff options
| -rw-r--r-- | chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java | 36 | ||||
| -rw-r--r-- | docs/chapter-backports.md | 23 |
2 files changed, 53 insertions, 6 deletions
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java index a9a83fb7..3c00b2e8 100644 --- a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java +++ b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java @@ -1,11 +1,37 @@ package com.seaofnodes.simple.node.cpus.x86_64_v2; -import com.seaofnodes.simple.node.Node; +import com.seaofnodes.simple.SB; +import com.seaofnodes.simple.node.*; +import com.seaofnodes.simple.codegen.*; -public class MulIX86 extends ImmX86 { - MulIX86( Node add, int imm ) { super(add,imm); } +public class MulIX86 extends MachConcreteNode implements MachNode { + final int _imm; + MulIX86( Node mul, int imm ) { + super(mul); + _inputs.pop(); // Pop the constant input + _imm = imm; // Record the constant + } @Override public String op() { return "muli"; } @Override public String glabel() { return "*"; } - @Override int opcode() { return 0x69; } - @Override int mod() { return 0; } + @Override public RegMask regmap(int i) { return x86_64_v2.RMASK; } + @Override public RegMask outregmap() { return x86_64_v2.WMASK; } + // Retain Chapter 21's allocation contract: output is also input #1. + @Override public int twoAddress() { return 1; } + + // IMUL encodes a destination register, unlike ImmX86's opcode extension. + @Override public final void encoding( Encoding enc ) { + short dst = enc.reg(this ); // Also src1 in this chapter + short src = enc.reg(in(1)); + enc.add1(x86_64_v2.rex(dst, src, 0)); + // opcode; 0x69 or 0x6B + enc.add1( 0x69 + (x86_64_v2.imm8(_imm) ? 2 : 0) ); + enc.add1( x86_64_v2.modrm(x86_64_v2.MOD.DIRECT, dst, src) ); + // immediate(4 bytes) 32 bits or (1 byte)8 bits + if( x86_64_v2.imm8(_imm) ) enc.add1(_imm); + else enc.add4(_imm); + } + // General form: "muli dst = src * #imm" + @Override public void asm(CodeGen code, SB sb) { + sb.p(code.reg(this)).p(" = ").p(code.reg(in(1))).p(" * #").p(_imm); + } } diff --git a/docs/chapter-backports.md b/docs/chapter-backports.md index 40717e5c..9714f697 100644 --- a/docs/chapter-backports.md +++ b/docs/chapter-backports.md @@ -20,7 +20,6 @@ the correction into the chapter's representation, not the entire modern file. | ID | Change | Proposed destination | Scope and acceptance evidence | |---|---|---|---| | B11 | Diagnose return types using the optimized return expression | 18; 19 already has the correction | `ReturnNode.err()` uses `expr()._type` instead of the parse-time `mt` aggregate. Chapter 18 rejects `struct S { u8 x; }; return new S; return 0;` with a mixed integer/reference error; Chapter 19 accepts it. Also test genuinely reachable incompatible returns. This is a candidate, not yet an applied or isolated-patch-verified fix. | -| B12 | Encode the actual destination register for two-address immediate multiply | 21 | `MulIX86` inherits `ImmX86.encoding`, whose ModRM.reg is fixed to zero and whose REX.R is clear. With source/destination both `rcx`, multiplying by 11 emits `48 6b c1 0b` (destination `rax`) instead of `48 6b c9 0b`. With both `r9`, it emits `49 6b c1 0b` instead of `4d 6b c9 0b`. Give multiply its proper register fields while preserving Chapter 21's two-address allocation contract; do not change the opcode-extension fields used by other `ImmX86` subclasses. Later chapters use a separate multiply encoder. | After the first two reviews, batch only corrections with established independence and regressions. Keep one logical correction per commit across affected chapters. @@ -131,6 +130,28 @@ Historical results below are dated evidence, not a substitute for a fresh baseline. Logs live in ignored build directories and may no longer exist. Reusable implementation lessons are in `skills/chapter25-codex-notes.md`. +### B12: immediate multiply destination corrected locally, 2026-09-19 + +The x86 backend has distinct integer register (`MulX86`, 0F AF), integer +immediate (`MulIX86`, 69/6B), and floating-point (`MulFX86`, MULSD) encoders. +Chapter 21 incorrectly routed immediate multiply through `ImmX86`, which +encodes an opcode extension in ModRM.reg. IMUL needs the destination there, +including its high bit in REX.R. Chapter 20 has no executable encoder yet; +22-25 already have a dedicated immediate multiply encoder. + +Chapter 21 now uses that dedicated encoder while preserving its +`twoAddress() == 1` allocation contract. Other `ImmX86` subclasses are unchanged. +For `rcx *= 11`, the regression failed with ModRM C1 instead of C9; the fix emits +`48 6b c9 0b`. For `r9 *= 11`, it emits `4d 6b c9 0b`. + +`Chapter21Test.testX86MultiplySameRegister` is carried through 21-25. It forces +rax/rcx/r9/r15 and positive/negative imm8/imm32 values, independently of allocator +choices, and checks register fields, opcode, immediate, length, and each +chapter's allocation contract. Existing distinct-register checks remain in +22-24. Unmodified baselines and final full `make tests` targets passed in all +five snapshots, with assertions enabled; Chapter 25 passed 446 tests. +Logs: `chapter25/build/b12-review/{baseline,red,fixed}.log`. + ### Issue #251: dead-node printing corrected locally, 2026-09-19 [Issue #251](https://github.com/SeaOfNodes/Simple/issues/251) identifies dead-node |