From 8331098d1904538a3a97d22e4d847cf2cc94e57d Mon Sep 17 00:00:00 2001 From: Cliff Click Date: Thu, 17 Sep 2026 15:04:32 -0700 Subject: x86 mul reg reg encoding fix --- chapter21/Makefile | 2 +- .../com/seaofnodes/simple/X86EncodingTest.java | 33 --------- .../seaofnodes/simple/codegen/X86EncodingTest.java | 32 +++++++++ chapter22/Makefile | 2 +- .../simple/node/cpus/x86_64_v2/MulIX86.java | 2 +- .../com/seaofnodes/simple/X86EncodingTest.java | 33 --------- .../seaofnodes/simple/codegen/X86EncodingTest.java | 79 ++++++++++++++++++++++ chapter23/Makefile | 2 +- .../simple/node/cpus/x86_64_v2/MulIX86.java | 2 +- .../com/seaofnodes/simple/X86EncodingTest.java | 33 --------- .../seaofnodes/simple/codegen/X86EncodingTest.java | 79 ++++++++++++++++++++++ chapter24/Makefile | 2 +- .../simple/node/cpus/x86_64_v2/MulIX86.java | 2 +- .../com/seaofnodes/simple/X86EncodingTest.java | 33 --------- .../seaofnodes/simple/codegen/X86EncodingTest.java | 79 ++++++++++++++++++++++ docs/chapter-backports.md | 13 ++-- 16 files changed, 280 insertions(+), 148 deletions(-) delete mode 100644 chapter21/src/test/java/com/seaofnodes/simple/X86EncodingTest.java create mode 100644 chapter21/src/test/java/com/seaofnodes/simple/codegen/X86EncodingTest.java delete mode 100644 chapter22/src/test/java/com/seaofnodes/simple/X86EncodingTest.java create mode 100644 chapter22/src/test/java/com/seaofnodes/simple/codegen/X86EncodingTest.java delete mode 100644 chapter23/src/test/java/com/seaofnodes/simple/X86EncodingTest.java create mode 100644 chapter23/src/test/java/com/seaofnodes/simple/codegen/X86EncodingTest.java delete mode 100644 chapter24/src/test/java/com/seaofnodes/simple/X86EncodingTest.java create mode 100644 chapter24/src/test/java/com/seaofnodes/simple/codegen/X86EncodingTest.java diff --git a/chapter21/Makefile b/chapter21/Makefile index 055c632a..429e40df 100644 --- a/chapter21/Makefile +++ b/chapter21/Makefile @@ -37,7 +37,7 @@ main_javas := $(wildcard $(SRC)/$(SIMPLE)/*java $(SRC)/$(SIMPLE)/*/*java $(SRC test_javas := $(wildcard $(TST)/$(SIMPLE)/*java $(TST)/$(SIMPLE)/*/*java) main_classes := $(patsubst $(SRC)/%java,$(CLZDIR)/main/%class,$(main_javas)) test_classes := $(patsubst $(TST)/%java,$(CLZDIR)/test/%class,$(test_javas)) -test_cp := $(patsubst $(TST)/$(SIMPLE)/%.java,com.seaofnodes.simple.%,$(wildcard $(TST)/$(SIMPLE)/*Test.java)) +test_cp := $(patsubst $(TST)/$(SIMPLE)/%.java,com.seaofnodes.simple.%,$(wildcard $(TST)/$(SIMPLE)/*Test.java)) com.seaofnodes.simple.codegen.X86EncodingTest classes = $(main_classes) $(test_classes) # All the libraries libs = $(wildcard lib/*jar) diff --git a/chapter21/src/test/java/com/seaofnodes/simple/X86EncodingTest.java b/chapter21/src/test/java/com/seaofnodes/simple/X86EncodingTest.java deleted file mode 100644 index 54fa96a8..00000000 --- a/chapter21/src/test/java/com/seaofnodes/simple/X86EncodingTest.java +++ /dev/null @@ -1,33 +0,0 @@ -package com.seaofnodes.simple; - -import com.seaofnodes.simple.codegen.CodeGen; -import com.seaofnodes.simple.node.cpus.x86_64_v2.CmpMemX86; -import org.junit.Test; -import static org.junit.Assert.*; - -public class X86EncodingTest { - @Test - public void testMemoryCompare64Immediate32() { - for (int immediate : new int[]{123456789, -123456789}) { - String src = "int[] !a = new int[2]; a[0] = arg; return a[arg & 1] == " + immediate + ";"; - CodeGen code = new CodeGen(src).driver(CodeGen.Phase.Encoding, "x86_64_v2", "SystemV"); - var cmp = code._stop.walk(n -> n instanceof CmpMemX86 ? n : null); - assertNotNull("Must exercise a memory comparison", cmp); - var enc = code._encoding; - int start = enc._opStart[cmp._nid]; - - // REX.W, 81 /7, SIB with scale 8, disp8, and a sign-extended imm32. - assertEquals(0x48, enc.read1(start) & 0xF8); - assertEquals(0x81, enc.read1(start + 1)); - assertEquals(0x7C, enc.read1(start + 2)); - assertEquals(0xC0, enc.read1(start + 3) & 0xC0); - assertEquals(8, enc.read1(start + 4)); - assertEquals(immediate, enc.read4(start + 5)); - assertEquals("CMP r/m64 uses four immediate bytes", 9, enc._opLen[cmp._nid]); - - // SETE must immediately follow the CPU's nine-byte CMP instruction. - assertEquals(0x0F, enc.read1(start + 9)); - assertEquals(0x94, enc.read1(start + 10)); - } - } -} diff --git a/chapter21/src/test/java/com/seaofnodes/simple/codegen/X86EncodingTest.java b/chapter21/src/test/java/com/seaofnodes/simple/codegen/X86EncodingTest.java new file mode 100644 index 00000000..03a57675 --- /dev/null +++ b/chapter21/src/test/java/com/seaofnodes/simple/codegen/X86EncodingTest.java @@ -0,0 +1,32 @@ +package com.seaofnodes.simple.codegen; + +import com.seaofnodes.simple.node.cpus.x86_64_v2.CmpMemX86; +import org.junit.Test; +import static org.junit.Assert.*; + +public class X86EncodingTest { + @Test + public void testMemoryCompare64Immediate32() { + for (int immediate : new int[]{123456789, -123456789}) { + String src = "int[] !a = new int[2]; a[0] = arg; return a[arg & 1] == " + immediate + ";"; + CodeGen code = new CodeGen(src).driver(CodeGen.Phase.Encoding, "x86_64_v2", "SystemV"); + var cmp = code._stop.walk(n -> n instanceof CmpMemX86 ? n : null); + assertNotNull("Must exercise a memory comparison", cmp); + var enc = code._encoding; + int start = enc._opStart[cmp._nid]; + + // REX.W, 81 /7, SIB with scale 8, disp8, and a sign-extended imm32. + assertEquals(0x48, enc.read1(start) & 0xF8); + assertEquals(0x81, enc.read1(start + 1)); + assertEquals(0x7C, enc.read1(start + 2)); + assertEquals(0xC0, enc.read1(start + 3) & 0xC0); + assertEquals(8, enc.read1(start + 4)); + assertEquals(immediate, enc.read4(start + 5)); + assertEquals("CMP r/m64 uses four immediate bytes", 9, enc._opLen[cmp._nid]); + + // SETE must immediately follow the CPU's nine-byte CMP instruction. + assertEquals(0x0F, enc.read1(start + 9)); + assertEquals(0x94, enc.read1(start + 10)); + } + } +} diff --git a/chapter22/Makefile b/chapter22/Makefile index deca5c76..55d5e70a 100644 --- a/chapter22/Makefile +++ b/chapter22/Makefile @@ -65,7 +65,7 @@ main_javas := $(wildcard $(SRC)/$(SIMPLE)/*java $(SRC)/$(SIMPLE)/*/*java $(SRC test_javas := $(wildcard $(TST)/$(SIMPLE)/*java $(TST)/$(SIMPLE)/*/*java) main_classes := $(patsubst $(SRC)/%java,$(CLZDIR)/main/%class,$(main_javas)) test_classes := $(patsubst $(TST)/%java,$(CLZDIR)/test/%class,$(test_javas)) -test_cp := $(patsubst $(TST)/$(SIMPLE)/%.java,com.seaofnodes.simple.%,$(wildcard $(TST)/$(SIMPLE)/*Test.java)) +test_cp := $(patsubst $(TST)/$(SIMPLE)/%.java,com.seaofnodes.simple.%,$(wildcard $(TST)/$(SIMPLE)/*Test.java)) com.seaofnodes.simple.codegen.X86EncodingTest classes = $(main_classes) $(test_classes) # All the libraries libs = $(wildcard lib/*jar) diff --git a/chapter22/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java b/chapter22/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java index 0dc16ab1..17006558 100644 --- a/chapter22/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java +++ b/chapter22/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java @@ -19,7 +19,7 @@ public class MulIX86 extends MachConcreteNode implements MachNode { @Override public final void encoding( Encoding enc ) { short dst = enc.reg(this ); // Also src1 short src = enc.reg(in(1)); - enc.add1(x86_64_v2.rex(src, dst, 0)); + 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) ); diff --git a/chapter22/src/test/java/com/seaofnodes/simple/X86EncodingTest.java b/chapter22/src/test/java/com/seaofnodes/simple/X86EncodingTest.java deleted file mode 100644 index 54fa96a8..00000000 --- a/chapter22/src/test/java/com/seaofnodes/simple/X86EncodingTest.java +++ /dev/null @@ -1,33 +0,0 @@ -package com.seaofnodes.simple; - -import com.seaofnodes.simple.codegen.CodeGen; -import com.seaofnodes.simple.node.cpus.x86_64_v2.CmpMemX86; -import org.junit.Test; -import static org.junit.Assert.*; - -public class X86EncodingTest { - @Test - public void testMemoryCompare64Immediate32() { - for (int immediate : new int[]{123456789, -123456789}) { - String src = "int[] !a = new int[2]; a[0] = arg; return a[arg & 1] == " + immediate + ";"; - CodeGen code = new CodeGen(src).driver(CodeGen.Phase.Encoding, "x86_64_v2", "SystemV"); - var cmp = code._stop.walk(n -> n instanceof CmpMemX86 ? n : null); - assertNotNull("Must exercise a memory comparison", cmp); - var enc = code._encoding; - int start = enc._opStart[cmp._nid]; - - // REX.W, 81 /7, SIB with scale 8, disp8, and a sign-extended imm32. - assertEquals(0x48, enc.read1(start) & 0xF8); - assertEquals(0x81, enc.read1(start + 1)); - assertEquals(0x7C, enc.read1(start + 2)); - assertEquals(0xC0, enc.read1(start + 3) & 0xC0); - assertEquals(8, enc.read1(start + 4)); - assertEquals(immediate, enc.read4(start + 5)); - assertEquals("CMP r/m64 uses four immediate bytes", 9, enc._opLen[cmp._nid]); - - // SETE must immediately follow the CPU's nine-byte CMP instruction. - assertEquals(0x0F, enc.read1(start + 9)); - assertEquals(0x94, enc.read1(start + 10)); - } - } -} diff --git a/chapter22/src/test/java/com/seaofnodes/simple/codegen/X86EncodingTest.java b/chapter22/src/test/java/com/seaofnodes/simple/codegen/X86EncodingTest.java new file mode 100644 index 00000000..389f0363 --- /dev/null +++ b/chapter22/src/test/java/com/seaofnodes/simple/codegen/X86EncodingTest.java @@ -0,0 +1,79 @@ +package com.seaofnodes.simple.codegen; + +import com.seaofnodes.simple.node.Node; +import com.seaofnodes.simple.node.cpus.x86_64_v2.CmpMemX86; +import com.seaofnodes.simple.node.cpus.x86_64_v2.MulIX86; +import org.junit.Test; +import static org.junit.Assert.*; + +public class X86EncodingTest { + + @Test + public void testMultiplyImmediateRegisters() { + // Destination, source, expected REX and ModRM. Cross the register-8 + // boundary in both directions, with low/low and high/high controls. + int[][] cases = {{1, 9, 0x49, 0xC9}, {9, 1, 0x4C, 0xC9}, + {1, 2, 0x48, 0xCA}, {9, 10, 0x4D, 0xCA}}; + for (int immediate : new int[]{11, -11, 123456789, -123456789}) { + CodeGen code = new CodeGen("return arg * " + immediate + ";") + .driver(CodeGen.Phase.Select, "x86_64_v2", "SystemV"); + var mul = code._stop.walk(n -> n instanceof MulIX86 ? n : null); + assertNotNull("Must exercise immediate multiply", mul); + for (int[] regs : cases) { + var enc = new FixedRegisterEncoding( + code, mul, regs[0], mul.in(1), regs[1]); + ((MulIX86)mul).encoding(enc); + boolean small = immediate == 11 || immediate == -11; + assertEquals("REX for dst=" + regs[0] + ", src=" + regs[1], regs[2], enc.read1(0)); + assertEquals(small ? 0x6B : 0x69, enc.read1(1)); + assertEquals(regs[3], enc.read1(2)); + assertEquals(small ? 4 : 7, enc._bits.size()); + assertEquals(immediate, small ? (byte)enc.read1(3) : enc.read4(3)); + } + } + } + @Test + public void testMemoryCompare64Immediate32() { + for (int immediate : new int[]{123456789, -123456789}) { + String src = "int[] !a = new int[2]; a[0] = arg; return a[arg & 1] == " + immediate + ";"; + CodeGen code = new CodeGen(src).driver(CodeGen.Phase.Encoding, "x86_64_v2", "SystemV"); + var cmp = code._stop.walk(n -> n instanceof CmpMemX86 ? n : null); + assertNotNull("Must exercise a memory comparison", cmp); + var enc = code._encoding; + int start = enc._opStart[cmp._nid]; + + // REX.W, 81 /7, SIB with scale 8, disp8, and a sign-extended imm32. + assertEquals(0x48, enc.read1(start) & 0xF8); + assertEquals(0x81, enc.read1(start + 1)); + assertEquals(0x7C, enc.read1(start + 2)); + assertEquals(0xC0, enc.read1(start + 3) & 0xC0); + assertEquals(8, enc.read1(start + 4)); + assertEquals(immediate, enc.read4(start + 5)); + assertEquals("CMP r/m64 uses four immediate bytes", 9, enc._opLen[cmp._nid]); + + // SETE must immediately follow the CPU's nine-byte CMP instruction. + assertEquals(0x0F, enc.read1(start + 9)); + assertEquals(0x94, enc.read1(start + 10)); + } + } + + /** Exercise instruction encoders with explicit registers, independent of allocator choices. */ + private static class FixedRegisterEncoding extends Encoding { + private final Node _dst, _src; + private final short _dstReg, _srcReg; + + FixedRegisterEncoding(CodeGen code, Node dst, int dstReg, Node src, int srcReg) { + super(code); + _dst = dst; + _src = src; + _dstReg = (short)dstReg; + _srcReg = (short)srcReg; + } + + @Override public short reg(Node n) { + if (n == _dst) return _dstReg; + if (n == _src) return _srcReg; + throw new AssertionError("Unexpected register lookup"); + } + } +} diff --git a/chapter23/Makefile b/chapter23/Makefile index deca5c76..55d5e70a 100644 --- a/chapter23/Makefile +++ b/chapter23/Makefile @@ -65,7 +65,7 @@ main_javas := $(wildcard $(SRC)/$(SIMPLE)/*java $(SRC)/$(SIMPLE)/*/*java $(SRC test_javas := $(wildcard $(TST)/$(SIMPLE)/*java $(TST)/$(SIMPLE)/*/*java) main_classes := $(patsubst $(SRC)/%java,$(CLZDIR)/main/%class,$(main_javas)) test_classes := $(patsubst $(TST)/%java,$(CLZDIR)/test/%class,$(test_javas)) -test_cp := $(patsubst $(TST)/$(SIMPLE)/%.java,com.seaofnodes.simple.%,$(wildcard $(TST)/$(SIMPLE)/*Test.java)) +test_cp := $(patsubst $(TST)/$(SIMPLE)/%.java,com.seaofnodes.simple.%,$(wildcard $(TST)/$(SIMPLE)/*Test.java)) com.seaofnodes.simple.codegen.X86EncodingTest classes = $(main_classes) $(test_classes) # All the libraries libs = $(wildcard lib/*jar) diff --git a/chapter23/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java b/chapter23/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java index 280b8185..1e9e708f 100644 --- a/chapter23/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java +++ b/chapter23/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java @@ -19,7 +19,7 @@ public class MulIX86 extends MachConcreteNode implements MachNode { @Override public final void encoding( Encoding enc ) { short dst = enc.reg(this ); // Also src1 short src = enc.reg(in(1)); - enc.add1(x86_64_v2.rex(src, dst, 0)); + 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) ); diff --git a/chapter23/src/test/java/com/seaofnodes/simple/X86EncodingTest.java b/chapter23/src/test/java/com/seaofnodes/simple/X86EncodingTest.java deleted file mode 100644 index 54fa96a8..00000000 --- a/chapter23/src/test/java/com/seaofnodes/simple/X86EncodingTest.java +++ /dev/null @@ -1,33 +0,0 @@ -package com.seaofnodes.simple; - -import com.seaofnodes.simple.codegen.CodeGen; -import com.seaofnodes.simple.node.cpus.x86_64_v2.CmpMemX86; -import org.junit.Test; -import static org.junit.Assert.*; - -public class X86EncodingTest { - @Test - public void testMemoryCompare64Immediate32() { - for (int immediate : new int[]{123456789, -123456789}) { - String src = "int[] !a = new int[2]; a[0] = arg; return a[arg & 1] == " + immediate + ";"; - CodeGen code = new CodeGen(src).driver(CodeGen.Phase.Encoding, "x86_64_v2", "SystemV"); - var cmp = code._stop.walk(n -> n instanceof CmpMemX86 ? n : null); - assertNotNull("Must exercise a memory comparison", cmp); - var enc = code._encoding; - int start = enc._opStart[cmp._nid]; - - // REX.W, 81 /7, SIB with scale 8, disp8, and a sign-extended imm32. - assertEquals(0x48, enc.read1(start) & 0xF8); - assertEquals(0x81, enc.read1(start + 1)); - assertEquals(0x7C, enc.read1(start + 2)); - assertEquals(0xC0, enc.read1(start + 3) & 0xC0); - assertEquals(8, enc.read1(start + 4)); - assertEquals(immediate, enc.read4(start + 5)); - assertEquals("CMP r/m64 uses four immediate bytes", 9, enc._opLen[cmp._nid]); - - // SETE must immediately follow the CPU's nine-byte CMP instruction. - assertEquals(0x0F, enc.read1(start + 9)); - assertEquals(0x94, enc.read1(start + 10)); - } - } -} diff --git a/chapter23/src/test/java/com/seaofnodes/simple/codegen/X86EncodingTest.java b/chapter23/src/test/java/com/seaofnodes/simple/codegen/X86EncodingTest.java new file mode 100644 index 00000000..389f0363 --- /dev/null +++ b/chapter23/src/test/java/com/seaofnodes/simple/codegen/X86EncodingTest.java @@ -0,0 +1,79 @@ +package com.seaofnodes.simple.codegen; + +import com.seaofnodes.simple.node.Node; +import com.seaofnodes.simple.node.cpus.x86_64_v2.CmpMemX86; +import com.seaofnodes.simple.node.cpus.x86_64_v2.MulIX86; +import org.junit.Test; +import static org.junit.Assert.*; + +public class X86EncodingTest { + + @Test + public void testMultiplyImmediateRegisters() { + // Destination, source, expected REX and ModRM. Cross the register-8 + // boundary in both directions, with low/low and high/high controls. + int[][] cases = {{1, 9, 0x49, 0xC9}, {9, 1, 0x4C, 0xC9}, + {1, 2, 0x48, 0xCA}, {9, 10, 0x4D, 0xCA}}; + for (int immediate : new int[]{11, -11, 123456789, -123456789}) { + CodeGen code = new CodeGen("return arg * " + immediate + ";") + .driver(CodeGen.Phase.Select, "x86_64_v2", "SystemV"); + var mul = code._stop.walk(n -> n instanceof MulIX86 ? n : null); + assertNotNull("Must exercise immediate multiply", mul); + for (int[] regs : cases) { + var enc = new FixedRegisterEncoding( + code, mul, regs[0], mul.in(1), regs[1]); + ((MulIX86)mul).encoding(enc); + boolean small = immediate == 11 || immediate == -11; + assertEquals("REX for dst=" + regs[0] + ", src=" + regs[1], regs[2], enc.read1(0)); + assertEquals(small ? 0x6B : 0x69, enc.read1(1)); + assertEquals(regs[3], enc.read1(2)); + assertEquals(small ? 4 : 7, enc._bits.size()); + assertEquals(immediate, small ? (byte)enc.read1(3) : enc.read4(3)); + } + } + } + @Test + public void testMemoryCompare64Immediate32() { + for (int immediate : new int[]{123456789, -123456789}) { + String src = "int[] !a = new int[2]; a[0] = arg; return a[arg & 1] == " + immediate + ";"; + CodeGen code = new CodeGen(src).driver(CodeGen.Phase.Encoding, "x86_64_v2", "SystemV"); + var cmp = code._stop.walk(n -> n instanceof CmpMemX86 ? n : null); + assertNotNull("Must exercise a memory comparison", cmp); + var enc = code._encoding; + int start = enc._opStart[cmp._nid]; + + // REX.W, 81 /7, SIB with scale 8, disp8, and a sign-extended imm32. + assertEquals(0x48, enc.read1(start) & 0xF8); + assertEquals(0x81, enc.read1(start + 1)); + assertEquals(0x7C, enc.read1(start + 2)); + assertEquals(0xC0, enc.read1(start + 3) & 0xC0); + assertEquals(8, enc.read1(start + 4)); + assertEquals(immediate, enc.read4(start + 5)); + assertEquals("CMP r/m64 uses four immediate bytes", 9, enc._opLen[cmp._nid]); + + // SETE must immediately follow the CPU's nine-byte CMP instruction. + assertEquals(0x0F, enc.read1(start + 9)); + assertEquals(0x94, enc.read1(start + 10)); + } + } + + /** Exercise instruction encoders with explicit registers, independent of allocator choices. */ + private static class FixedRegisterEncoding extends Encoding { + private final Node _dst, _src; + private final short _dstReg, _srcReg; + + FixedRegisterEncoding(CodeGen code, Node dst, int dstReg, Node src, int srcReg) { + super(code); + _dst = dst; + _src = src; + _dstReg = (short)dstReg; + _srcReg = (short)srcReg; + } + + @Override public short reg(Node n) { + if (n == _dst) return _dstReg; + if (n == _src) return _srcReg; + throw new AssertionError("Unexpected register lookup"); + } + } +} diff --git a/chapter24/Makefile b/chapter24/Makefile index deca5c76..55d5e70a 100644 --- a/chapter24/Makefile +++ b/chapter24/Makefile @@ -65,7 +65,7 @@ main_javas := $(wildcard $(SRC)/$(SIMPLE)/*java $(SRC)/$(SIMPLE)/*/*java $(SRC test_javas := $(wildcard $(TST)/$(SIMPLE)/*java $(TST)/$(SIMPLE)/*/*java) main_classes := $(patsubst $(SRC)/%java,$(CLZDIR)/main/%class,$(main_javas)) test_classes := $(patsubst $(TST)/%java,$(CLZDIR)/test/%class,$(test_javas)) -test_cp := $(patsubst $(TST)/$(SIMPLE)/%.java,com.seaofnodes.simple.%,$(wildcard $(TST)/$(SIMPLE)/*Test.java)) +test_cp := $(patsubst $(TST)/$(SIMPLE)/%.java,com.seaofnodes.simple.%,$(wildcard $(TST)/$(SIMPLE)/*Test.java)) com.seaofnodes.simple.codegen.X86EncodingTest classes = $(main_classes) $(test_classes) # All the libraries libs = $(wildcard lib/*jar) diff --git a/chapter24/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java b/chapter24/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java index 280b8185..1e9e708f 100644 --- a/chapter24/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java +++ b/chapter24/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MulIX86.java @@ -19,7 +19,7 @@ public class MulIX86 extends MachConcreteNode implements MachNode { @Override public final void encoding( Encoding enc ) { short dst = enc.reg(this ); // Also src1 short src = enc.reg(in(1)); - enc.add1(x86_64_v2.rex(src, dst, 0)); + 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) ); diff --git a/chapter24/src/test/java/com/seaofnodes/simple/X86EncodingTest.java b/chapter24/src/test/java/com/seaofnodes/simple/X86EncodingTest.java deleted file mode 100644 index 54fa96a8..00000000 --- a/chapter24/src/test/java/com/seaofnodes/simple/X86EncodingTest.java +++ /dev/null @@ -1,33 +0,0 @@ -package com.seaofnodes.simple; - -import com.seaofnodes.simple.codegen.CodeGen; -import com.seaofnodes.simple.node.cpus.x86_64_v2.CmpMemX86; -import org.junit.Test; -import static org.junit.Assert.*; - -public class X86EncodingTest { - @Test - public void testMemoryCompare64Immediate32() { - for (int immediate : new int[]{123456789, -123456789}) { - String src = "int[] !a = new int[2]; a[0] = arg; return a[arg & 1] == " + immediate + ";"; - CodeGen code = new CodeGen(src).driver(CodeGen.Phase.Encoding, "x86_64_v2", "SystemV"); - var cmp = code._stop.walk(n -> n instanceof CmpMemX86 ? n : null); - assertNotNull("Must exercise a memory comparison", cmp); - var enc = code._encoding; - int start = enc._opStart[cmp._nid]; - - // REX.W, 81 /7, SIB with scale 8, disp8, and a sign-extended imm32. - assertEquals(0x48, enc.read1(start) & 0xF8); - assertEquals(0x81, enc.read1(start + 1)); - assertEquals(0x7C, enc.read1(start + 2)); - assertEquals(0xC0, enc.read1(start + 3) & 0xC0); - assertEquals(8, enc.read1(start + 4)); - assertEquals(immediate, enc.read4(start + 5)); - assertEquals("CMP r/m64 uses four immediate bytes", 9, enc._opLen[cmp._nid]); - - // SETE must immediately follow the CPU's nine-byte CMP instruction. - assertEquals(0x0F, enc.read1(start + 9)); - assertEquals(0x94, enc.read1(start + 10)); - } - } -} diff --git a/chapter24/src/test/java/com/seaofnodes/simple/codegen/X86EncodingTest.java b/chapter24/src/test/java/com/seaofnodes/simple/codegen/X86EncodingTest.java new file mode 100644 index 00000000..389f0363 --- /dev/null +++ b/chapter24/src/test/java/com/seaofnodes/simple/codegen/X86EncodingTest.java @@ -0,0 +1,79 @@ +package com.seaofnodes.simple.codegen; + +import com.seaofnodes.simple.node.Node; +import com.seaofnodes.simple.node.cpus.x86_64_v2.CmpMemX86; +import com.seaofnodes.simple.node.cpus.x86_64_v2.MulIX86; +import org.junit.Test; +import static org.junit.Assert.*; + +public class X86EncodingTest { + + @Test + public void testMultiplyImmediateRegisters() { + // Destination, source, expected REX and ModRM. Cross the register-8 + // boundary in both directions, with low/low and high/high controls. + int[][] cases = {{1, 9, 0x49, 0xC9}, {9, 1, 0x4C, 0xC9}, + {1, 2, 0x48, 0xCA}, {9, 10, 0x4D, 0xCA}}; + for (int immediate : new int[]{11, -11, 123456789, -123456789}) { + CodeGen code = new CodeGen("return arg * " + immediate + ";") + .driver(CodeGen.Phase.Select, "x86_64_v2", "SystemV"); + var mul = code._stop.walk(n -> n instanceof MulIX86 ? n : null); + assertNotNull("Must exercise immediate multiply", mul); + for (int[] regs : cases) { + var enc = new FixedRegisterEncoding( + code, mul, regs[0], mul.in(1), regs[1]); + ((MulIX86)mul).encoding(enc); + boolean small = immediate == 11 || immediate == -11; + assertEquals("REX for dst=" + regs[0] + ", src=" + regs[1], regs[2], enc.read1(0)); + assertEquals(small ? 0x6B : 0x69, enc.read1(1)); + assertEquals(regs[3], enc.read1(2)); + assertEquals(small ? 4 : 7, enc._bits.size()); + assertEquals(immediate, small ? (byte)enc.read1(3) : enc.read4(3)); + } + } + } + @Test + public void testMemoryCompare64Immediate32() { + for (int immediate : new int[]{123456789, -123456789}) { + String src = "int[] !a = new int[2]; a[0] = arg; return a[arg & 1] == " + immediate + ";"; + CodeGen code = new CodeGen(src).driver(CodeGen.Phase.Encoding, "x86_64_v2", "SystemV"); + var cmp = code._stop.walk(n -> n instanceof CmpMemX86 ? n : null); + assertNotNull("Must exercise a memory comparison", cmp); + var enc = code._encoding; + int start = enc._opStart[cmp._nid]; + + // REX.W, 81 /7, SIB with scale 8, disp8, and a sign-extended imm32. + assertEquals(0x48, enc.read1(start) & 0xF8); + assertEquals(0x81, enc.read1(start + 1)); + assertEquals(0x7C, enc.read1(start + 2)); + assertEquals(0xC0, enc.read1(start + 3) & 0xC0); + assertEquals(8, enc.read1(start + 4)); + assertEquals(immediate, enc.read4(start + 5)); + assertEquals("CMP r/m64 uses four immediate bytes", 9, enc._opLen[cmp._nid]); + + // SETE must immediately follow the CPU's nine-byte CMP instruction. + assertEquals(0x0F, enc.read1(start + 9)); + assertEquals(0x94, enc.read1(start + 10)); + } + } + + /** Exercise instruction encoders with explicit registers, independent of allocator choices. */ + private static class FixedRegisterEncoding extends Encoding { + private final Node _dst, _src; + private final short _dstReg, _srcReg; + + FixedRegisterEncoding(CodeGen code, Node dst, int dstReg, Node src, int srcReg) { + super(code); + _dst = dst; + _src = src; + _dstReg = (short)dstReg; + _srcReg = (short)srcReg; + } + + @Override public short reg(Node n) { + if (n == _dst) return _dstReg; + if (n == _src) return _srcReg; + throw new AssertionError("Unexpected register lookup"); + } + } +} diff --git a/docs/chapter-backports.md b/docs/chapter-backports.md index bd7e08e5..b01afa10 100644 --- a/docs/chapter-backports.md +++ b/docs/chapter-backports.md @@ -9,16 +9,10 @@ For now, keep building SSA with incomplete types in Chapter 25. Moving that architecture earlier, or splitting Chapter 25, is deferred while small changes establish the review workflow. No renumbering is committed. -## Next review candidate +## Small changes -| ID | Small change | Destination | Evidence and regression | -|---|---|---|---| -| B02 | Correct immediate-multiply REX register order | 22, then 23-24; audit 21 separately | `MulIX86.encoding`: `rex(src,dst,0)` becomes `rex(dst,src,0)`. Exercise source and destination on opposite sides of register 8. Chapter 21 uses `ImmX86`, not this one-line implementation. | - -Review B02 before starting another candidate. Keep each correction's reduced -failure, smallest patch, and test results together for review. - -## Subsequent small changes +Keep each correction's reduced failure, smallest patch, and test results together +for review. An **audit** item needs an old-chapter reproducer before it is scheduled. Copy the correction into the chapter's representation, not the entire modern file. @@ -34,6 +28,7 @@ the correction into the chapter's representation, not the entire modern file. | B09 | Make diagnostic function lookup read-only | 18 onward; **audit** first mutating printer lookup | Separate `lookupFun` from cleanup-performing `link`. Verify printing preserves graph/linker state. Do not import FunPtrNode or compilation units for this fix. | | B10 | Preserve widening state in integer nonzero refinement | **Audit** first chapter with both widening and `nonZero`; no later than 24 | Chapter 25 `TypeInteger.nonZero` preserves `_widen`. Check lattice laws and loop refinement. Exclude TypeScalar, storage-type, and serialization changes. | | 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. -- cgit v1.2.3