From 1785d1e18c4fe5ede6c4b2a0b6893733c9139725 Mon Sep 17 00:00:00 2001 From: Nikodemus Siivola Date: Wed, 6 May 2009 13:27:52 +0000 Subject: [PATCH] 1.0.28.17: tn packing issues in ALLOCATE-VECTOR-ON-STACK on x86oids * If WORDS and LENGTH were packed in the same TN all manner of badness arose. Adjust lifetimes to make sure it doesn't happen, and try to pack LENGTH into EAX while at it. Gabor Melis and Paul Khuong did the figuring out what's wrong. --- NEWS | 2 ++ src/compiler/x86-64/alloc.lisp | 9 ++++----- src/compiler/x86/alloc.lisp | 8 ++++---- tests/dynamic-extent.impure.lisp | 12 ++++++++++++ version.lisp-expr | 2 +- 5 files changed, 23 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index 728ef57..6abc921 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,8 @@ * optimization: multidimensional array accesses in the absence of type information regarding array rank are approximately 10% faster due to open coding of ARRAY-RANK. + * bug fix: potential miscompilation of array stack allocation on x86 and + x86-64. (reported by Time Tossavainen) * bug fix: some forms of AND, OR, and COND resulted in expansions that could result in their subforms being treated as top level forms. (reported by James Knight) diff --git a/src/compiler/x86-64/alloc.lisp b/src/compiler/x86-64/alloc.lisp index 22b18a3..9e44853 100644 --- a/src/compiler/x86-64/alloc.lisp +++ b/src/compiler/x86-64/alloc.lisp @@ -90,12 +90,12 @@ (storew length result vector-length-slot other-pointer-lowtag)))) (define-vop (allocate-vector-on-stack) - (:args (type :scs (unsigned-reg)) - (length :scs (any-reg)) + (:args (type :scs (unsigned-reg) :to :save) + (length :scs (any-reg) :to :eval :target zero) (words :scs (any-reg) :target ecx)) (:temporary (:sc any-reg :offset ecx-offset :from (:argument 2)) ecx) - (:temporary (:sc any-reg :offset eax-offset :from (:argument 2)) zero) - (:temporary (:sc any-reg :offset edi-offset :from (:argument 0)) res) + (:temporary (:sc any-reg :offset eax-offset :from :eval) zero) + (:temporary (:sc any-reg :offset edi-offset) res) (:results (result :scs (descriptor-reg) :from :load)) (:arg-types positive-fixnum positive-fixnum @@ -122,7 +122,6 @@ (inst rep) (inst stos zero))) -(in-package "SB!VM") (define-vop (make-fdefn) (:policy :fast-safe) diff --git a/src/compiler/x86/alloc.lisp b/src/compiler/x86/alloc.lisp index fa97650..6e2561b 100644 --- a/src/compiler/x86/alloc.lisp +++ b/src/compiler/x86/alloc.lisp @@ -115,12 +115,12 @@ (storew length result vector-length-slot other-pointer-lowtag))))))) (define-vop (allocate-vector-on-stack) - (:args (type :scs (unsigned-reg immediate)) - (length :scs (any-reg)) + (:args (type :scs (unsigned-reg immediate) :to :save) + (length :scs (any-reg) :to :eval :target zero) (words :scs (any-reg) :target ecx)) (:temporary (:sc any-reg :offset ecx-offset :from (:argument 2)) ecx) - (:temporary (:sc any-reg :offset eax-offset :from (:argument 2)) zero) - (:temporary (:sc any-reg :offset edi-offset :from (:argument 0)) res) + (:temporary (:sc any-reg :offset eax-offset :from :eval) zero) + (:temporary (:sc any-reg :offset edi-offset) res) (:results (result :scs (descriptor-reg) :from :load)) (:arg-types positive-fixnum positive-fixnum diff --git a/tests/dynamic-extent.impure.lisp b/tests/dynamic-extent.impure.lisp index 7867bae..aba5a18 100644 --- a/tests/dynamic-extent.impure.lisp +++ b/tests/dynamic-extent.impure.lisp @@ -634,4 +634,16 @@ (declare (dynamic-extent z)) (print z) t))))) + +;;; On x86 and x86-64 upto 1.0.28.16 LENGTH and WORDS argument +;;; tns to ALLOCATE-VECTOR-ON-STACK could be packed in the same +;;; location, leading to all manner of badness. ...reproducing this +;;; reliably is hard, but this it at least used to break on x86-64. +(defun length-and-words-packed-in-same-tn (m) + (declare (optimize speed (safety 0) (debug 0) (space 0))) + (let ((array (make-array (max 1 m) :element-type 'fixnum))) + (declare (dynamic-extent array)) + (array-total-size array))) +(with-test (:name :length-and-words-packed-in-same-tn) + (assert (= 1 (length-and-words-packed-in-same-tn -3)))) diff --git a/version.lisp-expr b/version.lisp-expr index 254d249..b64c0f1 100644 --- a/version.lisp-expr +++ b/version.lisp-expr @@ -17,4 +17,4 @@ ;;; checkins which aren't released. (And occasionally for internal ;;; versions, especially for internal versions off the main CVS ;;; branch, it gets hairier, e.g. "0.pre7.14.flaky4.13".) -"1.0.28.16" +"1.0.28.17" -- 1.7.10.4