Ticket #1046 (closed defect: fixed)

Opened 21 months ago

Last modified 21 months ago

stack discipline and speculative PUSHes.

Reported by: gb Owned by:
Priority: normal Milestone:
Component: Compiler Version: trunk
Keywords: Cc:

Description

See  http://clozure.com/pipermail/openmcl-devel/2013-January/013968.html

Given

(defun baz (A G H)
  (LABELS ((%F14 (x y z)
             (IF (OR (< -1 -2 (DOTIMES (I 0 (FLET ((%F11 (F11-1 F11-2
                                                                F11-3) 48)) -16))
                                (RETURN-FROM %F14 5)))
                     nil)
               nil
               (COUNT (DPB (COUNT A '(16)) (BYTE 11 0) (RETURN-FROM %F14
                                                         1))
                      #(2)))))
    (%F14 1 0 0)))

calling

(baz 1 2 3)

in the REPL on x8664 reports that (non-volatile) registers were clobbered in the call.

This worked correctly in earlier releases; it seems to have broken around the time that the x86 backend started trying to "elide" speculative push/pop pairs.

Change History

comment:1 Changed 21 months ago by gb

  • Status changed from new to closed
  • Resolution set to fixed

(In [15564]) When "eliding pushes" on the vstack, we punt if any vinsn in the sequence between the push and pop changes or depends on the compiler's notion of vstack depth. When generating a function return sequence and optimizing for space, we try to reuse previously-generated sequences if (a) nvrs need to be restored and (b) the current vstack depth is = to the vstack depth at the point where the previously generated sequence was generated. (In other words, the branch to a previous label depends on the compiler's notion of vstack depth, and eliding a PUSH would invalidate that. It's not clear that trying to share POPJ sequences saves space (that depends on the size of the JMP and the size of the instructions used to restore NVRs), but if we're going to do this use a new POPJ-VIA-JUMP vinsn to branch to the earlier sequence; the vinsn attributes of that vinsn are equivalent to those of POPJ and this should scare the push-elision code out of eliding anything.

(That's likely not the best solution, but this fixes ticket:1046 in the trunk.)

The most likely symptom of getting this wrong is that NVRs may not be restored correctly. We don't have NVRs on x8632 or ARM, but we should probably recognize the fact that "push elision" and "popj folding" can have this kind of unexpected interaction.

Note: See TracTickets for help on using tickets.