Unexpected consing in COND forms when all clauses return a double float.

Bug #1398785 reported by Mark Cox
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
SBCL
Fix Released
Wishlist
Unassigned

Bug Description

The following function conses when the NUMBER argument is a double float.

    (defun test (number)
      (cond ((typep number 'double-float)
             number)
            ((typep number 'single-float)
             (coerce number 'double-float))
            (t
             1d0)))

    (time
     (dotimes (i 1000000)
       (test 5d0)))

The function no longer conses when the constant 1d0 in TEST is changed to an integer or the second clause is removed.

The problem occurs in SBCL version 9ae04408 and in sbcl-1.2.4 on hosts running OSX and Linux.

Thank you to stassats in #sbcl for producing a concise test case.

*features*

(:ALIEN-CALLBACKS :ANSI-CL :ASH-RIGHT-VOPS :BSD :C-STACK-IS-CONTROL-STACK
 :COMMON-LISP :COMPARE-AND-SWAP-VOPS :COMPLEX-FLOAT-VOPS :CYCLE-COUNTER :DARWIN
 :DARWIN9-OR-BETTER :FLOAT-EQL-VOPS :GENCGC :IEEE-FLOATING-POINT
 :INLINE-CONSTANTS :INODE64 :LINKAGE-TABLE :LITTLE-ENDIAN
 :MACH-EXCEPTION-HANDLER :MACH-O :MEMORY-BARRIER-VOPS :MULTIPLY-HIGH-VOPS
 :OS-PROVIDES-BLKSIZE-T :OS-PROVIDES-DLADDR :OS-PROVIDES-DLOPEN
 :OS-PROVIDES-PUTWC :OS-PROVIDES-SUSECONDS-T :PACKAGE-LOCAL-NICKNAMES
 :RAW-INSTANCE-INIT-VOPS :SB-AFTER-XC-CORE :SB-CORE-COMPRESSION :SB-DOC
 :SB-EVAL :SB-LDB :SB-PACKAGE-LOCKS :SB-SIMD-PACK :SB-SOURCE-LOCATIONS :SB-TEST
 :SB-THREAD :SB-UNICODE :SB-XREF-FOR-INTERNALS :SBCL
 :STACK-ALLOCATABLE-CLOSURES :STACK-ALLOCATABLE-FIXED-OBJECTS
 :STACK-ALLOCATABLE-LISTS :STACK-ALLOCATABLE-VECTORS
 :STACK-GROWS-DOWNWARD-NOT-UPWARD :SYMBOL-INFO-VOPS :UD2-BREAKPOINTS :UNIX
 :UNWIND-TO-FRAME-AND-CALL-VOP :X86-64)

Tags: optimization
Mark Cox (markcox80)
description: updated
description: updated
description: updated
description: updated
Revision history for this message
Paul Khuong (pvk) wrote :

Fixed in 18f437b Prefer a boxed representation for constants that are immediately returned.

Changed in sbcl:
status: New → Fix Committed
Revision history for this message
Stas Boukarev (stassats) wrote :

The original test case still conses

(defun test (number)
  (declare (type (or fixnum double-float single-float) number))
  (cond ((typep number 'double-float)
         number)
        ((typep number 'single-float)
         (coerce number 'double-float))
        ((typep number 'fixnum)
         (coerce number 'double-float))))

Changed in sbcl:
status: Fix Committed → Triaged
importance: Undecided → Wishlist
tags: added: optimization
Revision history for this message
Stas Boukarev (stassats) wrote :

As in, it's not about consing the constant 1d0, but that the whole cond is derived to be of type double float, so the CRETURN gets a double-float tn, which is then boxed.

Revision history for this message
Paul Khuong (pvk) wrote :

I already have patch ready for this one too. However, it's not clear to me that it's always a win. Representation selection is arguably making a defensible choice here: forcing the returned TN to be boxed means that each coerce branch gets its own boxing code sequence.

Revision history for this message
Stas Boukarev (stassats) wrote :

Could the already-double-float just jump into the return past boxing? I don't know how hard it is to convince the compiler.

Revision history for this message
Paul Khuong (pvk) wrote :

Hard.

Changed in sbcl:
status: Triaged → Fix Released
Stas Boukarev (stassats)
Changed in sbcl:
status: Fix Released → Triaged
Revision history for this message
Stas Boukarev (stassats) wrote :

ad3b42e67418f0f8c501ff87970ceae84fa97106

Changed in sbcl:
status: Triaged → Fix Committed
Stas Boukarev (stassats)
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.