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

Bug #1398785 reported by Mark Cox on 2014-12-03
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
SBCL
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)

Mark Cox (markcox80) on 2014-12-03
description: updated
description: updated
description: updated
description: updated
Paul Khuong (pvk) wrote :

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

Changed in sbcl:
status: New → Fix Committed
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
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.

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.

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.

Paul Khuong (pvk) wrote :

Hard.

Changed in sbcl:
status: Triaged → Fix Released
Stas Boukarev (stassats) on 2015-01-01
Changed in sbcl:
status: Fix Released → Triaged
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers