From 45a21054495eb9ee491995b15d26681c203cea9b Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 8 Jun 2009 21:15:52 -0500 Subject: [PATCH] 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. --- .../cfg/two-operand/two-operand.factor | 19 ++++++------- .../cpu/x86/assembler/assembler-tests.factor | 8 ++++++ basis/cpu/x86/assembler/assembler.factor | 28 +++++++++++-------- basis/cpu/x86/x86.factor | 2 +- 4 files changed, 34 insertions(+), 23 deletions(-) diff --git a/basis/compiler/cfg/two-operand/two-operand.factor b/basis/compiler/cfg/two-operand/two-operand.factor index a3a83b9d14..d30a02b0d3 100644 --- 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 203edf956e..a8c54fa65e 100644 --- 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 2b40aa2053..95b85ac2dd 100644 --- 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 b3cb5b56ec..15c54aa7d8 100644 --- 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 ;