bug#39573: [3.0.0] Compiler fails to optimize out side-effect-free expression

[Andy Wingo]

On Wed 12 Feb 2020 12:50, Ludovic Courtès <address@hidden> writes:

> Hello!
>
> Consider this loop:
>
>   (let loop ((n (expt 2 18))
>              (i 1))
>     (unless (zero? n)
>       (loop (- n 1)
>             (logior 0 (ash i 1)))))
>
> Guile 2.2 strips away the computation of ‘i’ (it cannot throw, has no
> side effects, and the result is unused):
>
[...]
> L1:
>   27    (handle-interrupts)                                   at (unknown 
> file):21:11
>   28    (sub/immediate 4 4 1)           
>   29    (br-if-u64-=-scm 3 4 #t -2)     ;; -> L1              at (unknown 
> file):18:11

Like specifically, it removes the logior call.

> However, 3.0.0 keeps the computation of ‘i’:
>
[...]
> L3:
>   53    (instrument-loop 139)           
>   55    (handle-interrupts)             
>   56    (call-scm<-scm-uimm 5 5 1 3)                          at (unknown 
> file):388:11
>   58    (call-scm<-scm-uimm 3 3 1 34)                         at (unknown 
> file):389:21
>   60    (call-scm<-scm-scm 3 4 3 10)                          at (unknown 
> file):389:11
>   62    (=? 5 4)                                              at (unknown 
> file):385:11
>   63    (jne -10)                       ;; -> L3

Hoo, we need to fix the disassembler to output something more sensible
than this :P  IP 56 appears to be the 1-, 58 is the lsh/immediate, and
60 is the logior.

> I’m not sure where the optimization should be taking place.  Perhaps
> it’s just a matter of amount-of-work threshold somewhere?

It's not an amount-of-work, that's only in peval which does nothing to
either of these (though it certainly could).

I took a look.  I just pushed something to make (logior 0 INT) reduce to
INT, but it doesn't remove the loop variable.

Then I thought it was surely dead code elimination that just wasn't
doing its thing.  The value is unused, so it must be that it thought
that the `ash' was effectful.  That `ash' gets compiled to
`lsh/immediate', which does indeed raise an exception if the operand
isn't an int; but here we prove that it is.  The problem was a missing
"type checker" implementation for lsh/immediate, a problem introduced in
the refactored compilation of `ash'.  So, fixed in git now:

L3:
  45    (instrument-loop 135)           
  47    (handle-interrupts)             
  48    (call-scm<-scm-uimm 5 5 1 3)                          at (unknown 
file):4:12
  50    (=? 5 4)                                              at (unknown 
file):3:12
  51    (jne -6)                        ;; -> L3

Cheers,

Andy