cpu.x86.assembler: IMUL2 instruction was busted for immediate operands

When given a register and an immediate, it would generate imul imm,dst,dst however the 64-bit prefix was generated wrong and if dst was an extended register only the first operand would be an extended register. To fix this, change IMUL2 to not work on immediates anymore, and added a new IMUL3 that takes a destination register, source register, and immediate. Also, change compiler.cfg.two-operand to not two-operandize %mul-imm, since this isn't needed anymore.
This fixes the sporadic benchmark.tuple-arrays crash on 64-bit machines.

diff --git a/basis/compiler/cfg/two-operand/two-operand.factor b/basis/compiler/cfg/two-operand/two-operand.factor
index a3a83b9d14a800dddfda00d56f822074237a1c20..d30a02b0d35ebc39388921ad88a2f791e0a14181 100644 (file)
--- a/basis/compiler/cfg/two-operand/two-operand.factor
+++ b/basis/compiler/cfg/two-operand/two-operand.factor
@@ -1,15 +1,15 @@
 ! Copyright (C) 2008, 2009 Slava Pestov.
 ! See http://factorcode.org/license.txt for BSD license.
-USING: accessors arrays kernel sequences make compiler.cfg.instructions
-compiler.cfg.rpo cpu.architecture ;
+USING: accessors kernel sequences make compiler.cfg.instructions
+compiler.cfg.local cpu.architecture ;
 IN: compiler.cfg.two-operand
 
 ! On x86, instructions take the form x = x op y
 ! Our SSA IR is x = y op z
 
-! We don't bother with ##add, ##add-imm or ##sub-imm since x86
-! has a LEA instruction which is effectively a three-operand
-! addition
+! We don't bother with ##add, ##add-imm, ##sub-imm or ##mul-imm
+! since x86 has LEA and IMUL instructions which are effectively
+! three-operand addition and multiplication, respectively.
 
 : make-copy ( dst src -- insn ) \ ##copy new-insn ; inline
 
@@ -34,7 +34,6 @@ M: ##not convert-two-operand*
 
 M: ##sub convert-two-operand* convert-two-operand/integer ;
 M: ##mul convert-two-operand* convert-two-operand/integer ;
-M: ##mul-imm convert-two-operand* convert-two-operand/integer ;
 M: ##and convert-two-operand* convert-two-operand/integer ;
 M: ##and-imm convert-two-operand* convert-two-operand/integer ;
 M: ##or convert-two-operand* convert-two-operand/integer ;
@@ -54,9 +53,7 @@ M: insn convert-two-operand* , ;
 
 : convert-two-operand ( cfg -- cfg' )
     two-operand? [
-        dup [
-            [
-                [ [ convert-two-operand* ] each ] V{ } make
-            ] change-instructions drop
-        ] each-basic-block
+        [ drop ]
+        [ [ [ convert-two-operand* ] each ] V{ } make ]
+        local-optimization
     ] when ;

diff --git a/basis/cpu/x86/assembler/assembler-tests.factor b/basis/cpu/x86/assembler/assembler-tests.factor
index 203edf956e31297f50922a9b9e2c93d6320ae9e0..a8c54fa65ea06308abbe4015e59e21341f403788 100644 (file)
--- a/basis/cpu/x86/assembler/assembler-tests.factor
+++ b/basis/cpu/x86/assembler/assembler-tests.factor
@@ -64,3 +64,11 @@ IN: cpu.x86.assembler.tests
 [ { HEX: 48 HEX: d3 HEX: e9 } ] [ [ RCX CL SHR ] { } make ] unit-test
 
 [ { HEX: f7 HEX: c1 HEX: d2 HEX: 04 HEX: 00 HEX: 00 } ] [ [ ECX 1234 TEST ] { } make ] unit-test
+
+[ { HEX: 4d HEX: 6b HEX: c0 HEX: 03 } ] [ [ R8 R8 3 IMUL3 ] { } make ] unit-test
+[ { HEX: 49 HEX: 6b HEX: c0 HEX: 03 } ] [ [ RAX R8 3 IMUL3 ] { } make ] unit-test
+[ { HEX: 4c HEX: 6b HEX: c0 HEX: 03 } ] [ [ R8 RAX 3 IMUL3 ] { } make ] unit-test
+[ { HEX: 48 HEX: 6b HEX: c1 HEX: 03 } ] [ [ RAX RCX 3 IMUL3 ] { } make ] unit-test
+[ { HEX: 48 HEX: 69 HEX: c1 HEX: 44 HEX: 03 HEX: 00 HEX: 00 } ] [ [ RAX RCX HEX: 344 IMUL3 ] { } make ] unit-test
+
+[ { 15 183 195 } ] [ [ EAX BX MOVZX ] { } make ] unit-test

diff --git a/basis/cpu/x86/assembler/assembler.factor b/basis/cpu/x86/assembler/assembler.factor
index 2b40aa2053f0b55779c64b97056966d397cbc531..95b85ac2ddc205ebf06b038765b96cbcc32550cb 100644 (file)
--- a/basis/cpu/x86/assembler/assembler.factor
+++ b/basis/cpu/x86/assembler/assembler.factor
@@ -1,8 +1,8 @@
 ! Copyright (C) 2005, 2009 Slava Pestov.
 ! See http://factorcode.org/license.txt for BSD license.
-USING: arrays io.binary kernel combinators
-kernel.private math namespaces make sequences words system layouts
-math.order accessors cpu.x86.assembler.syntax ;
+USING: arrays io.binary kernel combinators kernel.private math
+namespaces make sequences words system layouts math.order accessors
+cpu.x86.assembler.syntax ;
 IN: cpu.x86.assembler
 
 ! A postfix assembler for x86-32 and x86-64.
@@ -402,20 +402,26 @@ M: operand TEST OCT: 204 2-operand ;
 : SHR ( dst n -- ) BIN: 101 (SHIFT) ;
 : SAR ( dst n -- ) BIN: 111 (SHIFT) ;
 
-GENERIC: IMUL2 ( dst src -- )
-M: immediate IMUL2 swap dup reg-code t HEX: 68 3array immediate-1/4 ;
-M: operand IMUL2 OCT: 257 extended-opcode (2-operand) ;
+: IMUL2 ( dst src -- )
+    OCT: 257 extended-opcode (2-operand) ;
+
+: IMUL3 ( dst src imm -- )
+    dup fits-in-byte? [
+        [ swap HEX: 6a 2-operand ] dip 1,
+    ] [
+        [ swap HEX: 68 2-operand ] dip 4,
+    ] if ;
 
 : MOVSX ( dst src -- )
-    dup register-32? OCT: 143 OCT: 276 extended-opcode ?
-    over register-16? [ BIN: 1 opcode-or ] when
-    swapd
+    swap
+    over register-32? OCT: 143 OCT: 276 extended-opcode ?
+    pick register-16? [ BIN: 1 opcode-or ] when
     (2-operand) ;
 
 : MOVZX ( dst src -- )
+    swap
     OCT: 266 extended-opcode
-    over register-16? [ BIN: 1 opcode-or ] when
-    swapd
+    pick register-16? [ BIN: 1 opcode-or ] when
     (2-operand) ;
 
 ! Conditional move

diff --git a/basis/cpu/x86/x86.factor b/basis/cpu/x86/x86.factor
index b3cb5b56ec6b77b526bfe3038f933ed2c0a5c47a..15c54aa7d81503ce63b8a33df8e54880c5f798c6 100644 (file)
--- a/basis/cpu/x86/x86.factor
+++ b/basis/cpu/x86/x86.factor
@@ -108,7 +108,7 @@ M: x86 %add-imm [+] LEA ;
 M: x86 %sub     nip SUB ;
 M: x86 %sub-imm neg [+] LEA ;
 M: x86 %mul     nip swap IMUL2 ;
-M: x86 %mul-imm nip IMUL2 ;
+M: x86 %mul-imm IMUL3 ;
 M: x86 %and     nip AND ;
 M: x86 %and-imm nip AND ;
 M: x86 %or      nip OR ;