stack allocation failure

Bug #902351 reported by Nikodemus Siivola
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
SBCL
Fix Released
Medium
Unassigned

Bug Description

(defun foo (a)
  (let* ((l1 (blah a))
         (l2 (blah a))
         (foo (make-array l1 :element-type '(unsigned-byte 8)
                             :initial-element 4))
         (bar (make-array l1 :element-type '(unsigned-byte 8)
                             :initial-element 0))
         (hoge (make-array l2 :element-type '(unsigned-byte 8)
                              :initial-element 2))
;;; this one gets a "couldn't stack allocate" note:
         (piyo (make-array l2 :element-type '(unsigned-byte 8)
                              :initial-element 0)))
    (declare ((mod 32752) l1)
             (fixnum l2))
    (declare (dynamic-extent foo bar hoge piyo))
    (do-something-with foo bar hoge piyo)
    a))

Revision history for this message
Nikodemus Siivola (nikodemus) wrote :

There are two problems here:

* If you look at the disassembly, you see that actually none of the non-zero initialized vectors are stack allocated. So we're missing notes. This is due to the transform for fill accidentally muffling them.

* The second problem is that for specialized vectors the fill transform prevents stack allocation.

...so need to make the DX code smarter about examining functions. It can be helped a bit by adding :RESULT-ARG information for bashers (and making them return the sequence), but it will need to deal with multiple refs to the vector as well -- so in addition to :RESULT-ARG we need a :DX-ARGS annoration to at least VECTOR-LENGTH.

(Though it occurs to me that maybe all args to flushable functions are actually :DX-ARGS?)

Changed in sbcl:
assignee: nobody → Nikodemus Siivola (nikodemus)
Changed in sbcl:
status: Triaged → In Progress
Revision history for this message
Nikodemus Siivola (nikodemus) wrote :

commit 40bff32181a4d9b591ae2bac69bbee3bd77a82bc
Author: Nikodemus Siivola <email address hidden>
Date: Sat Dec 10 02:25:51 2011 +0200

    stack-allocatable fill-initialized specialized arrays

     I *think* we had this working earlier already, but it's been broken at least
     for a while now since there were no tests for it.

     Add a DEFKNOWN to the array byte bashers, providing the RESULT-ARG -- and
     make them return the sequence.

     Replace the unused and bitrotted UNSAFE IR1 attribute with its inverse:
     DX-SAFE, and use that togather with RESULT-ARG to allow multiple refs to
     potentially DX leafs. Still accept UNSAFE in DEFKNOWNs occurring in
     user-code, but ignore it and give a style-warning.

     For now, add DX-SAFE to LENGTH and VECTOR-LENGTH, which is enough for our
     purposes.

     Fixes lp#902351.

commit 2c84ef5c1d09f73857d64880fe8b889fc395c0d5
Author: Nikodemus Siivola <email address hidden>
Date: Sat Dec 10 01:12:27 2011 +0200

    remove MUFFLE-CONDITION from the FILL transform

      It hides failure to stack allocate results of a MAKE-ARRAY with non-zero
      initial-element and non-T element-type.

      Part of lp#902351.

Changed in sbcl:
assignee: Nikodemus Siivola (nikodemus) → nobody
status: In Progress → Fix Committed
Revision history for this message
Nikodemus Siivola (nikodemus) wrote :

This one still complains.

(defun foo (a)
  (let* ((l1 (blah a))
         (l2 (blah a))
         (foo (make-array l1 :element-type '(unsigned-byte 8)
                             :initial-element 4))
         (bar (make-array l1 :element-type '(unsigned-byte 8)
                             :initial-element 0))
         (hoge (make-array l2 :element-type '(unsigned-byte 8)
                              :initial-element 2))
;;; this one gets a "couldn't stack allocate" note:
         (piyo (make-array l2 :element-type '(unsigned-byte 8)
                              :initial-element 0)))
    (declare ((mod 32752) l1)
             (fixnum l2))
    (declare (dynamic-extent foo bar hoge piyo))
    (do-something-with foo bar hoge piyo)
    (do-something-with foo bar hoge piyo)
    a))

Changed in sbcl:
assignee: nobody → Nikodemus Siivola (nikodemus)
status: Fix Committed → In Progress
Revision history for this message
Nikodemus Siivola (nikodemus) wrote :

commit bd0c2b854688663c40a50a4453d668a7abfc96db
Author: Nikodemus Siivola <email address hidden>
Date: Sat Dec 10 23:01:19 2011 +0200

    stack-allocatable fill-initialized specialized arrays, take 2

      Really fix lp#902537.

      Turns out multiple references to the stack allocated vector complicate
      things /just/ sufficiently the that DX machinery can't keep up. (I because
      the code that allocates and initializes the vector isn't substituted at the
      use-site due to multiple references.)

      While it would be nice to make it smart enough to deal with
      non-let-converted WITH-ARRAY-DATA, that looks a bit tricky... so instead
      simplify things in the FILL transform when the vector is a simple-array.

Changed in sbcl:
assignee: Nikodemus Siivola (nikodemus) → nobody
status: In Progress → Fix Committed
Revision history for this message
3b (00003b) wrote :

Still not quite right i think, only works if dimension is a constant (either literal or declared something like (integer 10 10)). Looks like adding DX-SAFE to the DEFKNOWN for %CHECK-BOUND fixes it, no idea if it breaks anything else though...

Revision history for this message
Nikodemus Siivola (nikodemus) wrote :

Doing that should be OK. I'll look into it tomorrow.

Changed in sbcl:
assignee: nobody → Nikodemus Siivola (nikodemus)
status: Fix Committed → In Progress
Revision history for this message
Nikodemus Siivola (nikodemus) wrote :

commit ddc81e75502cdc07a7f846644c2ae8e64a31b6d8
Author: Nikodemus Siivola <email address hidden>
Date: Thu Dec 29 20:32:31 2011 +0200

    stack-allocatable fill-initialized specialized arrays, part tres

     lp#902351

     Mark %CHECK-BOUND as DX-SAFE, so that vectors of unknown size can be stack
     allocated.

Changed in sbcl:
assignee: Nikodemus Siivola (nikodemus) → nobody
status: In Progress → Fix Committed
Changed in sbcl:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.