Compiler notes in FFLOOR

Bug #1903512 reported by Michał "phoe" Herda
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
SBCL
Fix Released
Wishlist
Unassigned

Bug Description

SBCL 2.0.10 on Linux amd64.

(defun fmod (number divisor)
  (declare (optimize speed) (double-float number divisor))
  (nth-value 1 (ffloor number divisor)))

This generates several "deleting unreachable code" notes and the following note that mentions that the double-float gets boxed:

; --> BLOCK MULTIPLE-VALUE-BIND MULTIPLE-VALUE-CALL FTRUNCATE BLOCK
; --> MACROLET SB-KERNEL::NUMBER-DISPATCH BLOCK TAGBODY RETURN-FROM
; --> TYPECASE LET COND IF IF IF PROGN TYPECASE LET COND IF PROGN
; --> SB-KERNEL::FTRUNCATE-FLOAT LET* SB-KERNEL:%UNARY-FTRUNCATE
; ==>
; (SB-C::%UNARY-FTRUNCATE/DOUBLE SB-C::X)
;
; note: doing float to pointer coercion (cost 13)

Revision history for this message
Christophe Rhodes (csr21-cantab) wrote : Re: [Bug 1903512] [NEW] Compiler notes in FFLOOR

Michał Herda <email address hidden> writes:

> (defun fmod (number divisor)
> (declare (optimize speed) (double-float number divisor))
> (nth-value 1 (ffloor number divisor)))
>
> This generates several "deleting unreachable code" notes and the
> following note that mentions that the double-float gets boxed:

I think the return-value boxing note is unavoidable. The large number
of unreachable code deletion notes are because the heuristic for whether
a deletion note is interesting to the user or not works quite badly if
both the user (you) and the compiler use the same variable names.
Normally the package system protects you from that, but when some
variables are named by symbols in the CL package there's the likelihood
of collision.

If you rename your first argument N (or something that isn't cl:number),
most of the notes go away. (SBCL should almost certainly not use
exported variable names like this in its transforms, which would make
this much less visible to users).

Revision history for this message
Michał "phoe" Herda (phoe-krk) wrote :

OK, let's change the variable names (thanks for the explanation!) and get rid of the implicit boxing by introducing an explicit array holding the unboxed double-float.

This leaves us with this core issue:

(defun fmod (num div)
  (declare (optimize speed) (double-float num div))
  (make-array 1 :element-type 'double-float
                :initial-contents (list (nth-value 1 (ffloor num div)))))

; file: /tmp/slimeZppxvS
; in: DEFUN FMOD
; (FFLOOR NUM DIV)
; --> BLOCK MULTIPLE-VALUE-BIND MULTIPLE-VALUE-CALL FTRUNCATE BLOCK
; --> MACROLET SB-KERNEL::NUMBER-DISPATCH BLOCK TAGBODY RETURN-FROM
; --> TYPECASE LET COND IF IF IF PROGN TYPECASE LET COND IF PROGN
; --> SB-KERNEL::FTRUNCATE-FLOAT LET* SB-KERNEL:%UNARY-FTRUNCATE
; ==>
; (SB-C::%UNARY-FTRUNCATE/DOUBLE SB-C::X)
;
; note: doing float to pointer coercion (cost 13)
;
; compilation unit finished
; printed 1 note

Revision history for this message
Douglas Katzman (dougk) wrote :

the code fragment has another problem now - it must materialize a list as input to make-array, so it has to box the float to put it in a list. (An SSC could always avoid making the list, but that's another issue. Sometimes we can, sometimes we can't)

The only way to prove that the notes are coming from part of the emitted code that definitely have to do with floating-point math where you've excluded - the extent possible - any other ostensible need to materialize a boxed would would be to rewrite the test case as something like this:
  (let ((a (make-array 1 :element-type 'double-float)))
     (setf (aref a 0) (nth-value 1 (ffloor num div))))
  a) ; and don't return the float, of course

That said, there is still float boxing because we're not actually able to inline ffloor.

So while the original report was about "unreachable code", the new note is about boxing because of the (OPTIMIZE SPEED) declaration.
This kind of complaint might broadly fall under a bug for making the compiler able to inline more floating-point operations. I'm not sure if you want to keep this open, but certainly the original subject line of "notes in FFLOOR" doesn't really convey that. And the reason we can't inline the floor operation per se is that ... well, the algorithm is crappy, to a certain degree.
Improvements to it are welcome. It might be interesting to see how the performance compares to the code emitted by a C compiler using trunc() plus a subtraction to obtain the remainder.

Changed in sbcl:
importance: Undecided → Wishlist
Revision history for this message
Stas Boukarev (stassats) wrote :

I don't see any notes anymore.

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