Bogus code generated in low DEBUG
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
SBCL |
Fix Released
|
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)
(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::
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
Changed in sbcl: | |
status: | Triaged → Fix Committed |
Changed in sbcl: | |
status: | Fix Committed → Fix Released |
A slight reduction of the above. Call it with (PREPARE #(3))
(defun prepare (context)
(outer- index 3))
(g . h)
(o . p)
#((i . j) (k . l) (m . n)))
outer-index) ))
(when (eq payload nil) ; always false
(unwind- protect t))))
(declare (optimize (debug 1) (safety 0)))
(let ((npcs (the fixnum (svref context 0)))
(tagbody
outer-loop
(let ((payload
(svref #(#((a . b) (c . d) (e . f))
(loop for jj fixnum from 0 below (if (listp payload) 1 npcs)
do (print jj)
;; the mere presence of this unwind-protect borks things
(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
(#: LOOP-LIMIT- 0 (IF (LISTP PAYLOAD) 1 NPCS))))
start stack:
52> entry NIL
53> 54: SB-C::CLAMBDA (LET ((JJ 0)
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
( #:LOOP- LIMIT-0 (IF (LISTP PAYLOAD) 1 NPCS)))
) :KIND :LET
start stack:
65> local combination v54 v56 <none>
66> bind SB-C::CLAMBDA (LET ((JJ 0)
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
(#:LOOP- LIMIT-0 (IF (LISTP PAYLOAD) 1 NPCS))))
start stack:
52> entry NIL
53> 54: SB-C::CLAMBDA (LET ((JJ 0)
55> 56: '0
57> 58: LISTP {GLOBAL-FUNCTION}
59> 60: PAYLOAD
61> 62: known combination v58 v60
63> if ...