Re: [Chicken-hackers] [PATCH] Mostly fix #1604

[Peter Bex]

On Sat, May 18, 2019 at 08:46:40PM +0300, megane wrote:
> Here's what I figured out fwiw:
>   ...
>   (let ((g234 n10))
>     (let ((g235 0))
>       (k144 (##core#inline "C_eqp" g234 g235))))
>   ...
> 
> This transformation comes from this rewrite rule:
>   (rewrite 'scheme#= 9 "C_eqp" "C_i_equalp" #t #t)
> 
> I don't know why the temporary variables are needed here. They seem to
> be the source of the sub-optimal optimization.

They're needed because of the fold-boolean:

(= a b c) => (let ((x a) (y b) (z c))
               (##core#inline "C_and" (##core_inline "C_eqp" x y)
                                      (##core_inline "C_eqp" y z)))

The y is repeated here.  If b is an expression with side-effects, it
would be evaluated twice.  That's why the let is needed.  Normally the
let can be dropped, but the compiler marks procedure arguments as
"captured", so in the "fib" case, the let must be kept.  I'm not sure
exactly why; but I think it has something to do with the possibility
that the outer variable is mutated.  Maybe Felix can weigh in on this?
If possible we could try to distinguish between user let and compiler-
introduced temporary lets, so that the compiler knows capturing can't
possibly make a difference?

The relevant code for dropping lets in the optimizer is the (let)
handling in perform-high-level-optimizations, lines 264 to 279.
The code that determines if it's replacable is in analyze-expression
in core.scm, lines 2374 to 2397.

Attached is a patch that avoids binding the first and the last arguments
to a variable in this particular rewrite rule.  This has the effect of
emitting an empty let (which can be dropped) when there are exactly two
arguments.  I checked, and the generated C of both (= ...) and (eq? ...)
is now identical (after also applying my other patch from earlier in the
thread), modulo a few gensym variables' numbers.

Cheers,
Peter

0001-Do-not-inject-unnecessary-let-with-extra-variables-i.patch
Description: Text Data

signature.asc
Description: PGP signature