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
From afb982b07a7fe3c8386b9414a37d70783543aede Mon Sep 17 00:00:00 2001
From: Peter Bex <pe...@more-magic.net>
Date: Sun, 26 May 2019 22:19:26 +0200
Subject: [PATCH] Do not inject unnecessary let with extra variables in
 multi-arg boolean rewrite

Instead of expanding (= a b) to
(let ((x a) (y b)) (##core#inline "C_eqp" x y)), we expand to
(##core#inline "C_eqp" a b)

The extra let would cause a fast procedure call to be split up into
CPS when the arguments are captured (this prevents the let from
being dropped by the optimizer).  We can only do this for the first
and the last argument, because the intermediate ones are used twice
in the expansion:

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

This was found while working on #1604.
---
 optimizer.scm | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/optimizer.scm b/optimizer.scm
index abca38df..fa9d5850 100644
--- a/optimizer.scm
+++ b/optimizer.scm
@@ -1057,11 +1057,17 @@
 	      (and (or (and unsafe (not (eq? number-type 'generic)))
 		       (and (eq? number-type 'fixnum) (third classargs))
 		       (and (eq? number-type 'flonum) (fourth classargs)) )
-		   (let* ((names (map (lambda (z) (gensym)) callargs))
-			  (vars (map varnode names)) )
-		     (let loop ((callargs callargs)
+		   ;; First and last arg are kept as-is because the
+		   ;; injected let can't always be dropped.
+		   (let* ((names (map (lambda (z) (gensym))
+				      (take (cdr callargs)
+					    (- (length callargs) 2))))
+			  (vars (append (cons (car callargs)
+					      (map varnode names))
+					(list (last callargs)))))
+		     (let loop ((callargs (cdr callargs))
 				(names names))
-		       (if (null? callargs)
+		       (if (null? names)
 			   (make-node
 			    '##core#call (list #t)
 			    (list 
-- 
2.11.0

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Reply via email to