x86 mul reg reg encoding fix

4 files changed, 81 insertions, 35 deletions

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");
+        }
+    }
+}