From d93593470bc65b36d9a6d7e833dd387ba4f4dd1d Mon Sep 17 00:00:00 2001 From: Douglas Katzman Date: Sat, 9 Jan 2016 22:56:43 -0500 Subject: [PATCH] Correct the x86-64 CRC32 instruction definition. The emitter was buggy if the destination size was :QWORD and the source size was not, or if the source size was :WORD. And the printers were very buggy. --- src/compiler/disassem.lisp | 7 +++--- src/compiler/x86-64/insts.lisp | 54 ++++++++++++++++++++++++++++++------------ tests/assembler.pure.lisp | 26 ++++++++++++++++++++ 3 files changed, 68 insertions(+), 19 deletions(-) diff --git a/src/compiler/disassem.lisp b/src/compiler/disassem.lisp index ddf047e42..a4f283c42 100644 --- a/src/compiler/disassem.lisp +++ b/src/compiler/disassem.lisp @@ -631,10 +631,6 @@ ;;; final target system (defun modify-or-add-arg (arg-name args &rest properties) (declare (dynamic-extent properties)) - (when (get-properties properties '(:field :fields)) - (error "~@" - arg-name)) (let* ((cell (member arg-name args :key #'arg-name)) (arg (if cell (setf (car cell) (copy-structure (car cell))) @@ -670,6 +666,9 @@ (when use-label-p (setf (arg-use-label arg) use-label)) (when fields-p + (awhen (and (null format-length) *current-instruction-flavor*) + (setq format-length ; FORMAT-LENGTH is not (FORMAT-LENGTH ...). wtf? + (bytes-to-bits (format-length (format-or-lose (cdr it)))))) (setf (arg-fields arg) (mapcar (lambda (bytespec) (when (> (+ (byte-position bytespec) (byte-size bytespec)) diff --git a/src/compiler/x86-64/insts.lisp b/src/compiler/x86-64/insts.lisp index ea0662d5a..a1d9f86aa 100644 --- a/src/compiler/x86-64/insts.lisp +++ b/src/compiler/x86-64/insts.lisp @@ -137,6 +137,17 @@ stream dstate)) +;; Print a reg that can only be a :DWORD or :QWORD. +;; Avoid use of INST-OPERAND-SIZE because it's wrong for this type of operand. +(defun print-d/q-word-reg (value stream dstate) + (declare (type full-reg value) + (type stream stream) + (type disassem-state dstate)) + (print-reg-with-width value + (if (dstate-get-inst-prop dstate 'rex-w) :qword :dword) + stream + dstate)) + (defun print-byte-reg (value stream dstate) (declare (type full-reg value) (type stream stream) @@ -3738,21 +3749,34 @@ (emit-sse-inst segment dst src #xf3 #xb8))) (define-instruction crc32 (segment dst src) - (:printer-list - `(,@(mapcan (lambda (op2) - (mapcar (lambda (instfmt) - `(,instfmt ((prefix (#xf2)) (op1 (#x38)) - (op2 (,op2))))) - '(ext-rex-2byte-prefix-reg-reg/mem - ext-2byte-prefix-reg-reg/mem))) - '(#xf0 #xf1)))) - (:emitter - (let ((dst-size (operand-size dst))) - (aver (and (register-p dst) (not (or (eq dst-size :word) - (eq dst-size :byte))))) - (if (eq (operand-size src) :byte) - (emit-sse-inst-2byte segment dst src #xf2 #x38 #xf0) - (emit-sse-inst-2byte segment dst src #xf2 #x38 #xf1))))) + ;; The low bit of the final opcode byte sets the source size. + ;; REX.W bit sets the destination size. can't have #x66 prefix and REX.W = 1. + (:printer ext-2byte-prefix-reg-reg/mem + ((prefix #xf2) (op1 #x38) + (op2 #b1111000 :field (byte 7 25)) ; #xF0 ignoring the low bit + (src-width nil :field (byte 1 24) :prefilter #'prefilter-width) + (reg nil :printer #'print-d/q-word-reg))) + (:printer ext-rex-2byte-prefix-reg-reg/mem + ((prefix #xf2) (op1 #x38) + (op2 #b1111000 :field (byte 7 33)) ; ditto + (src-width nil :field (byte 1 32) :prefilter #'prefilter-width) + (reg nil :printer #'print-d/q-word-reg))) + (:emitter + (let ((dst-size (operand-size dst)) + (src-size (operand-size src))) + ;; The following operand size combinations are possible: + ;; dst = r32, src = r/m{8, 16, 32} + ;; dst = r64, src = r/m{8, 64} + (aver (and (register-p dst) + (memq src-size (case dst-size + (:dword '(:byte :word :dword)) + (:qword '(:byte :qword)))))) + (maybe-emit-operand-size-prefix segment src-size) + (emit-sse-inst-2byte segment dst src #xf2 #x38 + (if (eq src-size :byte) #xf0 #xf1) + ;; :OPERAND-SIZE is ordinarily determined + ;; from 'src', so override it to use 'dst'. + :operand-size dst-size)))) ;;;; Miscellany diff --git a/tests/assembler.pure.lisp b/tests/assembler.pure.lisp index 52550396c..e77ba0507 100644 --- a/tests/assembler.pure.lisp +++ b/tests/assembler.pure.lisp @@ -42,6 +42,32 @@ (test-movnti (make-ea :qword :base rax-tn) r12-tn "4C0FC320 MOVNTI [RAX], R12"))) +(with-test (:name :assemble-crc32 :skipped-on '(not :x86-64)) + ;; Destination size = :DWORD + (test-assemble `(crc32 ,eax-tn ,(make-ea :byte :base rbp-tn)) + "F20F38F04500 CRC32 EAX, BYTE PTR [RBP]") + (test-assemble `(crc32 ,eax-tn ,(make-ea :word :base rbp-tn)) + "66F20F38F14500 CRC32 EAX, WORD PTR [RBP]") + (test-assemble `(crc32 ,eax-tn ,(make-ea :dword :base rbp-tn)) + "F20F38F14500 CRC32 EAX, DWORD PTR [RBP]") + ;; these check that the presence of REX does not per se change the width. + (test-assemble `(crc32 ,r9d-tn ,(make-ea :byte :base r14-tn :index r15-tn)) + "F2470F38F00C3E CRC32 R9D, BYTE PTR [R14+R15]") + (test-assemble `(crc32 ,r9d-tn ,(make-ea :word :base r14-tn :index r15-tn)) + "66F2470F38F10C3E CRC32 R9D, WORD PTR [R14+R15]") + (test-assemble `(crc32 ,r9d-tn ,(make-ea :dword :base r14-tn :index r15-tn)) + "F2470F38F10C3E CRC32 R9D, DWORD PTR [R14+R15]") + ;; Destination size = :QWORD + (test-assemble `(crc32 ,rax-tn ,(make-ea :byte :base rbp-tn)) + "F2480F38F04500 CRC32 RAX, BYTE PTR [RBP]") + (test-assemble `(crc32 ,rax-tn ,(make-ea :qword :base rbp-tn)) + "F2480F38F14500 CRC32 RAX, QWORD PTR [RBP]") + ;; now with high regs + (test-assemble `(crc32 ,r9-tn ,(make-ea :byte :base r14-tn :index r15-tn)) + "F24F0F38F00C3E CRC32 R9, BYTE PTR [R14+R15]") + (test-assemble `(crc32 ,r9-tn ,(make-ea :qword :base r14-tn :index r15-tn)) + "F24F0F38F10C3E CRC32 R9, QWORD PTR [R14+R15]")) + (with-test (:name :disassemble-arith-insts :skipped-on '(not (or :x86 :x86-64))) (flet ((try (inst expect) (let ((p (search "$fp" expect))) -- 2.11.4.GIT