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.

diff --git a/NEWS b/NEWS
index 728ef57..6abc921 100644 (file)
--- 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 (file)
--- 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")
 \f
 (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 (file)
--- 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 (file)
--- 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))))
 \f

diff --git a/version.lisp-expr b/version.lisp-expr
index 254d249..b64c0f1 100644 (file)
--- 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"