Bogus code generated in low DEBUG

Bug #1446891 reported by Douglas Katzman on 2015-04-21
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
SBCL
High
Unassigned

Bug Description

Given:

(defstruct (root-ctxt (:copier nil) (:predicate nil)) (npcs 0 :type fixnum))

(defun prepare (context data)
  (declare (optimize (debug 1)))
  (let ((npcs (root-ctxt-npcs context)))
    (loop for payload across (the simple-vector data)
          do (print payload)
       ;; If PAYLOAD has the shape (X . Y) then process it.
       ;; Otherwise it is a vector of things of that shape each to be processed.
       ;; In the loop below, "process" is actually code that never executes.
       (loop for i fixnum from 0 below (if (listp payload) 1 npcs)
             do (princ i)
                (write-char #\space)
                (let ((thing
                       (if (listp payload)
                           payload
                           (svref (sb-ext:truly-the simple-vector payload) i))))
                  (when (eq thing nil) ; always false
                    ;; the mere presence of this unwind-protect borks things
                    (unwind-protect (eval ''foo) (print 'cleanup))))))))

(defglobal *some-data* ; vector of 4 things, a few of which are nested vectors
  #(#((a . b) (c . d) (e . f))
    (g . h)
    #((i . j) (k . l) (m . n))
    (o . p)))

This call to PREPARE should iterate over 4 things in the outer loop and for the first and third outer iteration, iterate over 3 things in the inner loop. Instead the third outer iteration only executes one inner iteration.

* (prepare (make-root-ctxt :npcs 3) *some-data*)
#((A . B) (C . D) (E . F)) 0 1 2
(G . H) 0
#((I . J) (K . L) (M . N)) 0
(O . P) 0

The third line should have printed '0 1 2' at the end.
I suspect that the compiler thinks that NPCS is a single-use variable.
This can be seen by adding (declare (optimize (sb-c::preserve-single-use-debug-variables 3)))
or changing DEBUG to 2, or - as someone here discovered to workaround the issue - spuriously touch the variable anywhere else.
This makes the nested loops run correctly:
#((A . B) (C . D) (E . F)) 0 1 2
(G . H) 0
#((I . J) (K . L) (M . N)) 0 1 2
(O . P) 0

Douglas Katzman (dougk) wrote :
Download full text (3.9 KiB)

A slight reduction of the above. Call it with (PREPARE #(3))

(defun prepare (context)
  (declare (optimize (debug 1) (safety 0)))
  (let ((npcs (the fixnum (svref context 0)))
        (outer-index 3))
    (tagbody
      outer-loop
      (let ((payload
             (svref #(#((a . b) (c . d) (e . f))
                              (g . h)
                              (o . p)
                              #((i . j) (k . l) (m . n)))
                            outer-index)))
        (loop for jj fixnum from 0 below (if (listp payload) 1 npcs)
            do (print jj)
               (when (eq payload nil) ; always false
                 ;; the mere presence of this unwind-protect borks things
                 (unwind-protect t))))
       (unless (minusp (decf outer-index)) (go outer-loop)))))

You can see the problem from this fragment of the trace file.
lvar 30 holds the [incorrect] result of (IF (LISTP PAYLOAD) 1 NPCS).
But lvar 30 has only a single WRITE into it. The write for the ELSE branch of the IF is missing, and the ELSE code jumps directly to the top of the loop.
Also note that the lambda-let for (loop for jj ...) elides the binding of #:loop-limit-0 and reads directly from lvar 30, which would have been OK if lvar 30 were correct.

IR1 block 5 start c52
start stack:
 52> entry NIL
 53> 54: SB-C::CLAMBDA (LET ((JJ 0)
                                                            (#:LOOP-LIMIT-0 (IF (LISTP PAYLOAD) 1 NPCS))))
 55> 56: '0
 57> 58: LISTP {GLOBAL-FUNCTION}
 59> 60: PAYLOAD
 61> 62: known combination v58 v60
 63> if v62 c64 c65
end stack:
successors c65 c64

IR1 block 19 start c64
start stack:
 64> 30: '1
end stack:
successors c65

IR1 block 6 start c65
start stack:
 65> local combination v54 v56 <none>
 66> bind SB-C::CLAMBDA (LET ((JJ 0)
                                  (#:LOOP-LIMIT-0 (IF (LISTP PAYLOAD) 1 NPCS)))
                              ) :KIND :LET
end stack:
successors c67

IR1 block 7 start c67
start stack:
 67> entry NIL
end stack:
successors c68

IR1 block 8 start c68
start stack:
 68> 69: JJ
 70> 71: < {GLOBAL-FUNCTION}
 72> 73: known combination v71 v69 v30 ; <<< BUG. v30 was only written by one branch of the IF.
 74> if v73 c75 c76
end stack:
successors c76 c75

---
For comparison's sake, here is the code when you artificially inhibit single-use lvar elimination with (DEBUG 2) or whatever.
Lvar 66 holds the result of (IF (LISTP PAYLOAD) 1 NPCS). It is written from both successor blocks of the IF, as expected.
The comparison of JJ is against the lambda var #:LOOP-LIMIT-0. Though I think it would have been legal to elide the lambda var and read from lvar 66 which was correctly computed.

IR1 block 5 start c52
start stack:
 52> entry NIL
 53> 54: SB-C::CLAMBDA (LET ((JJ 0)
                             (#:LOOP-LIMIT-0 (IF (LISTP PAYLOAD) 1 NPCS))))
 55> 56: '0
 57> 58: LISTP {GLOBAL-FUNCTION}
 59> 60: PAYLOAD
 61> 62: known combination v58 v60
 63> if ...

Read more...

Stas Boukarev (stassats) wrote :

Reduced to
(defun test (a)
  (declare (optimize (debug 0) (safety 0)))
  (let ((a (the fixnum a))
        (x 1)
        z)
    (tagbody
     loop
       (let ((y (if (= x 1) #xFFF a)))
         (declare (type fixnum y))
         (block nil
           (flet ((empty ())
                  (ret () (return)))
             (setf z y)
             (empty)
             (ret))))
       (unless (= x 0)
         (setf x 0)
         (go loop)))
    z))

(test 2) => 4095

Changed in sbcl:
status: New → Triaged
importance: Undecided → High
Changed in sbcl:
status: Triaged → Fix Committed
Changed in sbcl:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers