bug#30020: floating point unboxing regression in 2.2.3

[Mark H Weaver <mhw@netris.org>]

Hi David,

Sorry for the long delay.

"Thompson, David" <dthompson2@worcester.edu> writes:
> Guile 2.2.3 seems to have lost some of the abilities that Guile 2.2.2
> had wrt unboxing floats.  Here's a simple procedure to show the
> problem. It simply adds the first two elements of an f32vector:
>
>     (define (add-two-floats bv)
>       (+ (f32vector-ref bv 0) (f32vector-ref bv 1)))

Thanks for the tiny test case.  This helps a lot!

> Here's the disassembly from 2.2.2 (note that f64->scm appears only once):
>
>     Disassembly of #<procedure add-two-floats (bv)> at #x7efef4006230:
>
>        0    (assert-nargs-ee/locals 2 1)    ;; 3 slots (1 arg)    at
> (unknown file):22:0
>        1    (load-u64 2 0 0)                                      at
> (unknown file):23:26
>        4    (bv-f32-ref 2 1 2)
>        5    (load-u64 0 0 4)                                      at
> (unknown file):23:47
>        8    (bv-f32-ref 1 1 0)
>        9    (fadd 2 2 1)                                          at
> (unknown file):23:23
>       10    (f64->scm 1 2)
>       11    (handle-interrupts)
>       12    (return-values 2)               ;; 1 value
>
> And here is 2.2.3:
>
>     Disassembly of #<procedure add-two-floats (bv)> at #x2457140:
>
>        0    (assert-nargs-ee/locals 2 1)    ;; 3 slots (1 arg)    at
> (unknown file):29:0
>        1    (load-u64 2 0 0)                                      at
> (unknown file):30:26
>        4    (bv-f32-ref 2 1 2)
>        5    (f64->scm 2 2)
>        6    (load-u64 0 0 4)                                      at
> (unknown file):30:47
>        9    (bv-f32-ref 1 1 0)
>       10    (f64->scm 1 1)
>       11    (scm->f64 2 2)                                        at
> (unknown file):30:23
>       12    (scm->f64 1 1)
>       13    (fadd 2 2 1)
>       14    (f64->scm 1 2)
>       15    (handle-interrupts)
>       16    (return-values 2)               ;; 1 value
>
> 2.2.3 is reading unboxed floats from the bytevector, boxing them,
> unboxing them, then calling fadd.  The result is that some formerly
> well optimized code of mine that generated little to no garbage is now
> generating lots of garbage.

I did the moral equivalent of a git bisect, and found the culprit:

  commit d4883307ca64a7028b9a6cd072974437306c19d3
  Author: Andy Wingo <wingo@pobox.com>
  Date:   Thu Nov 30 10:41:45 2017 +0100

  Minor CSE run-time optimization

  * module/language/cps/cse.scm (compute-equivalent-subexpressions): Minor
  optimization to reduce the size of equivalent expression keys, and to
  avoid some work if an expression has no key.

Fortunately, this commit can be reverted in isolation without any
difficulty, and apparently without any negative consequences.  If a
better solution isn't found soon, perhaps that's what we should do.

Andy, do you have any idea what's going on here?

     Thanks,
       Mark