Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-10-07 Thread megane

Peter Bex  writes:

> On Sat, Jul 20, 2019 at 11:51:28AM +0300, megane wrote:
>> Here's a new patch that drops the (not captured) check.
>
> Thanks for making that!  Now that my original patch has been pushed,
> here's an incremental patch based on yours which only drops that check.
>
> Cheers,
> Peter

Thanks, pushed.

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


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-10-06 Thread felix . winkelmann
> On Sun, Sep 15, 2019 at 01:16:50PM +0200, Peter Bex wrote:
> > On Sun, Aug 25, 2019 at 04:34:02PM +0200, Peter Bex wrote:
> > > On Sat, Jul 20, 2019 at 11:51:28AM +0300, megane wrote:
> > > > Here's a new patch that drops the (not captured) check.
> > > 
> > > Thanks for making that!  Now that my original patch has been pushed,
> > > here's an incremental patch based on yours which only drops that check.
> > 
> > Hi all,
> > 
> > How do you feel about this latter patch?
> 
> I'd like to close the ticket, can we apply the final patch too?

Well, give it a try and let's see what salmonella reports.


fleix


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


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-10-06 Thread Peter Bex
On Sun, Sep 15, 2019 at 01:16:50PM +0200, Peter Bex wrote:
> On Sun, Aug 25, 2019 at 04:34:02PM +0200, Peter Bex wrote:
> > On Sat, Jul 20, 2019 at 11:51:28AM +0300, megane wrote:
> > > Here's a new patch that drops the (not captured) check.
> > 
> > Thanks for making that!  Now that my original patch has been pushed,
> > here's an incremental patch based on yours which only drops that check.
> 
> Hi all,
> 
> How do you feel about this latter patch?

I'd like to close the ticket, can we apply the final patch too?

Cheers,
Peter


signature.asc
Description: PGP signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-09-15 Thread Peter Bex
On Sun, Aug 25, 2019 at 04:34:02PM +0200, Peter Bex wrote:
> On Sat, Jul 20, 2019 at 11:51:28AM +0300, megane wrote:
> > Here's a new patch that drops the (not captured) check.
> 
> Thanks for making that!  Now that my original patch has been pushed,
> here's an incremental patch based on yours which only drops that check.

Hi all,

How do you feel about this latter patch?

Cheers,
Peter


signature.asc
Description: PGP signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-08-25 Thread Peter Bex
On Sat, Jul 20, 2019 at 11:51:28AM +0300, megane wrote:
> Here's a new patch that drops the (not captured) check.

Thanks for making that!  Now that my original patch has been pushed,
here's an incremental patch based on yours which only drops that check.

Cheers,
Peter
From 03000b61cfd13bffe809c0908b2144f86a4f49f0 Mon Sep 17 00:00:00 2001
From: Peter Bex 
Date: Sun, 25 Aug 2019 16:23:11 +0200
Subject: [PATCH] Also allow captured variables with known values from being
 replaced

This should still be safe.  Only when the variable is assigned to is
this not allowed.

This change should completely fix situations like in #1620
---
 core.scm | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/core.scm b/core.scm
index bd36448b..2fce485b 100644
--- a/core.scm
+++ b/core.scm
@@ -2368,21 +2368,24 @@
 			undefined) )
 	   (quick-put! plist 'removable #t) )
 
-	 ;; Make 'replacable, if it has a variable as known value and if either that variable has
-	 ;;  a known value itself, or the target and the source are never assigned and the source
-	 ;;  is non-global or we are in block-mode:
-	 ;;  - The target-variable is not allowed to be global.
+	 ;; Make 'replacable, if
+	 ;; - it has a variable as known value and
+	 ;; - it is not a global
+	 ;; - it is never assigned to and
+	 ;; - if either the substitute has a known value itself or
+	 ;;   * the substitute is never assigned to and
+	 ;;   * we are in block-mode or the substitute is non-global
+	 ;;
 	 ;;  - The variable that can be substituted for the current one is marked as 'replacing.
 	 ;;This is done to prohibit beta-contraction of the replacing variable (It wouldn't be there, if
 	 ;;it was contracted).
 	 (when (and value (not global))
 	   (when (eq? '##core#variable (node-class value))
 	 (let ((name (first (node-parameters value))) )
-	   (when (and (not captured)
+	   (when (and (not assigned)
 			  (or (and (not (db-get db name 'unknown))
    (db-get db name 'value))
-			  (and (not assigned)
-   (not (db-get db name 'assigned))
+			  (and (not (db-get db name 'assigned))
    (or (not (variable-visible?
 	 name block-compilation))
    (not (db-get db name 'global))) ) ))
-- 
2.20.1



signature.asc
Description: PGP signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-07-20 Thread megane

Peter Bex  writes:

> On Thu, Jul 11, 2019 at 03:15:00PM +0300, megane wrote:
>> Of course if this is dropped the other conditions must still meet.
>>
[...]
>>
>> Here's a correct way to drop the (not captured) check:
>>
>>   (and (not assigned)
>>(or (and (not (db-get db name 'unknown))
>> (db-get db name 'value))
>>(and (not (db-get db name 'assigned))
>> (or (not (variable-visible?
>>   name block-compilation))
>> (not (db-get db name 'global))) ) ))
>
> Wow, nice catch!  That makes a lot of sense.  I'll cook up a proper
> patch unless someone beats me to it.
>

Here's a new patch that drops the (not captured) check.

I also updated the comment and formatted it so it's easier to change in
the future if needed.

>From dbf207a90e47265cbcbca0a82c010710003a6dc2 Mon Sep 17 00:00:00 2001
From: Peter Bex 
Date: Sun, 30 Jun 2019 15:42:19 +0200
Subject: [PATCH] Mark aliased variable as replacable even if either variable
 is captured

The only thing that really matters is whether it is global or assigned
to, the capture state is irrelevant as far as I can tell.

Fixes #1620
---
 core.scm | 31 ++-
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/core.scm b/core.scm
index f74b140f..1468819d 100644
--- a/core.scm
+++ b/core.scm
@@ -2317,11 +2317,9 @@
 (quick-put! plist 'inlinable #t)
 (quick-put! plist 'local-value n
 
-;; Make 'collapsable, if it has a known constant value which is either 
collapsable or is only
-;;  referenced once and if no assignments are made:
-(when (and value
-   ;; (not (assq 'assigned plist)) - If it has a known value, 
it's assigned just once!
-   (eq? 'quote (node-class value)) )
+;; Make 'collapsable, if it has a known constant value which
+;; is either collapsable or is only referenced once:
+(when (and value (eq? 'quote (node-class value)) )
   (let ((val (first (node-parameters value
 (when (or (collapsable-literal? val)
   (= 1 nreferences) )
@@ -2371,25 +2369,24 @@
undefined) )
   (quick-put! plist 'removable #t) )
 
-;; Make 'replacable, if it has a variable as known value and if either 
that variable has
-;;  a known value itself, or if it is not captured and referenced only 
once, the target and
-;;  the source are never assigned and the source is non-global or we 
are in block-mode:
-;;  - The target-variable is not allowed to be global.
+;; Make 'replacable, if
+;; - it has a variable as known value and
+;; - it is not a global
+;; - it is never assigned to and
+;; - if either the substitute has a known value itself or
+;;   * the substitute is never assigned to and
+;;   * we are in block-mode or the substitute is non-global
+;;
 ;;  - The variable that can be substituted for the current one is 
marked as 'replacing.
 ;;This is done to prohibit beta-contraction of the replacing 
variable (It wouldn't be there, if
 ;;it was contracted).
 (when (and value (not global))
   (when (eq? '##core#variable (node-class value))
-(let* ((name (first (node-parameters value)))
-   (nrefs (db-get db name 'references)) )
-  (when (and (not captured)
+(let ((name (first (node-parameters value))) )
+  (when (and (not assigned)
  (or (and (not (db-get db name 'unknown))
   (db-get db name 'value))
- (and (not (db-get db name 'captured))
-  nrefs
-  (= 1 (length nrefs))
-  (not assigned)
-  (not (db-get db name 'assigned))
+ (and (not (db-get db name 'assigned))
   (or (not (variable-visible?
 name block-compilation))
   (not (db-get db name 'global))) ) ))
-- 
2.17.1

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


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-07-11 Thread Peter Bex
On Thu, Jul 11, 2019 at 03:15:00PM +0300, megane wrote:
> Of course if this is dropped the other conditions must still meet.
> 
> Here's your proposed condition:
> 
>   (and (not captured)
>(or (and (not (db-get db name 'unknown))
> (db-get db name 'value))
>(and (not assigned)
> (not (db-get db name 'assigned))
> (or (not (variable-visible?
>   name block-compilation))
> (not (db-get db name 'global))) )
>   ))
> 
> 
> If the (not captured) part is just dropped then if the first branch of
> the or is taken, namely this:
> 
>   (and (not (db-get db name 'unknown))
>(db-get db name 'value))
> 
> then it's not checked that the variable to be replaced is never assigned
> to! Here's a case that triggers the error:
> 
>   (define (foo x)
> (letrec ((pe* (lambda () (print x) (pe*
>   (pe*)))
>   (foo '((foo1 e)))
>   (foo 'foo2)
> 
> The letrec expands to an assignment and to a (##core#undefined) that
> triggers the first branch of the or.
> 
> Here's a correct way to drop the (not captured) check:
> 
>   (and (not assigned)
>(or (and (not (db-get db name 'unknown))
> (db-get db name 'value))
>(and (not (db-get db name 'assigned))
> (or (not (variable-visible?
>   name block-compilation))
> (not (db-get db name 'global))) ) ))

Wow, nice catch!  That makes a lot of sense.  I'll cook up a proper
patch unless someone beats me to it.

Cheers,
Peter


signature.asc
Description: PGP signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-07-11 Thread megane

megane  writes:

> Peter Bex  writes:
>
> Regarding the capturing, the capture test is still there:
>
>
>>   (when (and value (not global))
>> (when (eq? '##core#variable (node-class value))
>> - (let* ((name (first (node-parameters value)))
>> -(nrefs (db-get db name 'references)) )
>> + (let ((name (first (node-parameters value))) )
>> (when (and (not captured)
>   ^^
>   Here.
> I got the impression you wanted to remove this.
>

Of course if this is dropped the other conditions must still meet.

Here's your proposed condition:

  (and (not captured)
   (or (and (not (db-get db name 'unknown))
(db-get db name 'value))
   (and (not assigned)
(not (db-get db name 'assigned))
(or (not (variable-visible?
  name block-compilation))
(not (db-get db name 'global))) )
  ))


If the (not captured) part is just dropped then if the first branch of
the or is taken, namely this:

  (and (not (db-get db name 'unknown))
   (db-get db name 'value))

then it's not checked that the variable to be replaced is never assigned
to! Here's a case that triggers the error:

  (define (foo x)
(letrec ((pe* (lambda () (print x) (pe*
  (pe*)))
  (foo '((foo1 e)))
  (foo 'foo2)

The letrec expands to an assignment and to a (##core#undefined) that
triggers the first branch of the or.

Here's a correct way to drop the (not captured) check:

  (and (not assigned)
   (or (and (not (db-get db name 'unknown))
(db-get db name 'value))
   (and (not (db-get db name 'assigned))
(or (not (variable-visible?
  name block-compilation))
(not (db-get db name 'global))) ) ))

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


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-07-05 Thread Peter Bex
On Fri, Jul 05, 2019 at 09:13:36AM +0300, megane wrote:
> I reduced this case to this:
> 
>   (define (foo bindings)
> (define (append-map proc lst1)
>   (if lst1
>   (proc 1)
>   (proc 1 2)))
> (append-map (lambda (b a) (begin)) bindings))
> 
> Error: ../fail.scm:5: proc: procedure `proc' called with wrong number of
> arguments

Wow, my mind is blown.  Great job at finding this!  This seems to be
exactly what is happening inside the compiler after removing the
(not captured) test, but like you show here it doesn't have anything
to do with that test, it can be triggered separately too.

I created a ticket for it: https://bugs.call-cc.org/ticket/1630

> This seems to be a separate optimizer issue as it triggers with 5.1.0rc1
> and 4.13.0 too.

I think you're right, this has nothing to do with the patch.  We'll need
to fix this bug first, I think, and then we can try again with removing
the (not captured) check.

Cheers,
Peter


signature.asc
Description: PGP signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-07-04 Thread megane

Peter Bex  writes:

> On Wed, Jul 03, 2019 at 02:05:21PM +0200, Peter Bex wrote:
>> You're right, good catch!  That was an oversight on my part, I only
>> removed the captured check of the other variable.  I hope this makes
>> things faster in more cases.  I can make and test a new patch, but don't
>> know when I'll get around to it.  Possibly in the weekend.
>
> I tried this, but I got a crash when compiling CHICKEN with itself after
> having built it with this patch.
>
> I'm not even sure why it's doing this.  The offending procedure was
> append-map from mini-srfi-1, it's calling proc with the wrong number
> of arguments.
>

I reduced this case to this:

  (define (foo bindings)
(define (append-map proc lst1)
  (if lst1
  (proc 1)
  (proc 1 2)))
(append-map (lambda (b a) (begin)) bindings))

Error: ../fail.scm:5: proc: procedure `proc' called with wrong number of
arguments

This seems to be a separate optimizer issue as it triggers with 5.1.0rc1
and 4.13.0 too.

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


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-07-04 Thread megane

megane  writes:

> Peter Bex  writes:
>
>> On Wed, Jul 03, 2019 at 02:05:21PM +0200, Peter Bex wrote:
>>> You're right, good catch!  That was an oversight on my part, I only
>>> removed the captured check of the other variable.  I hope this makes
>>> things faster in more cases.  I can make and test a new patch, but don't
>>> know when I'll get around to it.  Possibly in the weekend.
>>
>> I tried this, but I got a crash when compiling CHICKEN with itself after
>> having built it with this patch.
>
> Do you mean you tried with the '(not captured)' check removed?
>
>>
>> I'm not even sure why it's doing this.  The offending procedure was
>> append-map from mini-srfi-1, it's calling proc with the wrong number
>> of arguments.
>
> There's this in the definition of append-map:
>
> (define (append-map proc lst1 . lsts)
>   ...
>   (append (proc x) r)
>
>Calling proc with 1 argument
>
> The error was:
>
> Error: mini-srfi-1.scm:72: proc: procedure `proc' called with wrong number of 
> arguments
> rules.make:831: recipe for target 'chicken-ffi-syntax.c' failed
>
> In chicken-ffi-syntax.scm in the definition for let-syntax there's this:
>
>(append-map
> (lambda (b a)
>  
>  This expects 2 arguments!!!
>   (if (pair? (cddr b))
>   (list (cons a (cddr b)))
>   '() ) )
>   bindings aliases)

Sorry, ignore the above.. I wasn't thinking about what append-map was supposed
to do..

I'll take a closer look after some sleep.

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


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-07-04 Thread megane

Peter Bex  writes:

> On Wed, Jul 03, 2019 at 02:05:21PM +0200, Peter Bex wrote:
>> You're right, good catch!  That was an oversight on my part, I only
>> removed the captured check of the other variable.  I hope this makes
>> things faster in more cases.  I can make and test a new patch, but don't
>> know when I'll get around to it.  Possibly in the weekend.
>
> I tried this, but I got a crash when compiling CHICKEN with itself after
> having built it with this patch.

Do you mean you tried with the '(not captured)' check removed?

>
> I'm not even sure why it's doing this.  The offending procedure was
> append-map from mini-srfi-1, it's calling proc with the wrong number
> of arguments.

There's this in the definition of append-map:

(define (append-map proc lst1 . lsts)
  ...
  (append (proc x) r)
   
   Calling proc with 1 argument

The error was:

Error: mini-srfi-1.scm:72: proc: procedure `proc' called with wrong number of 
arguments
rules.make:831: recipe for target 'chicken-ffi-syntax.c' failed

In chicken-ffi-syntax.scm in the definition for let-syntax there's this:

   (append-map
  (lambda (b a)
 
 This expects 2 arguments!!!
(if (pair? (cddr b))
(list (cons a (cddr b)))
'() ) )
bindings aliases)
>
> I *think* the reason is that you can't replace variables which are
> formal arguments to user procedures, because then all the calls will
> have the wrong number of arguments.
>
> Currently we don't seem to mark formal arguments in any special way,
> so checking if they're captured seems to be the best way.  So my initial
> patch is fine, but perhaps we can refine it in some way by adding this
> distinction?
>
> Cheers,
> Peter


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


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-07-04 Thread Peter Bex
On Wed, Jul 03, 2019 at 02:05:21PM +0200, Peter Bex wrote:
> You're right, good catch!  That was an oversight on my part, I only
> removed the captured check of the other variable.  I hope this makes
> things faster in more cases.  I can make and test a new patch, but don't
> know when I'll get around to it.  Possibly in the weekend.

I tried this, but I got a crash when compiling CHICKEN with itself after
having built it with this patch.

I'm not even sure why it's doing this.  The offending procedure was
append-map from mini-srfi-1, it's calling proc with the wrong number
of arguments.

I *think* the reason is that you can't replace variables which are
formal arguments to user procedures, because then all the calls will
have the wrong number of arguments.

Currently we don't seem to mark formal arguments in any special way,
so checking if they're captured seems to be the best way.  So my initial
patch is fine, but perhaps we can refine it in some way by adding this
distinction?

Cheers,
Peter


signature.asc
Description: PGP signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-07-03 Thread Peter Bex
On Wed, Jul 03, 2019 at 02:54:24PM +0300, megane wrote:
> Regarding the capturing, the capture test is still there:
> 
> >  (when (and value (not global))
> >(when (eq? '##core#variable (node-class value))
> > -(let* ((name (first (node-parameters value)))
> > -   (nrefs (db-get db name 'references)) )
> > +(let ((name (first (node-parameters value))) )
> >(when (and (not captured)
>   ^^
>   Here.
> I got the impression you wanted to remove this.

You're right, good catch!  That was an oversight on my part, I only
removed the captured check of the other variable.  I hope this makes
things faster in more cases.  I can make and test a new patch, but don't
know when I'll get around to it.  Possibly in the weekend.

Cheers,
Peter


signature.asc
Description: PGP signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-07-03 Thread megane

Peter Bex  writes:

> Hi all,
>
> I had a look at #1620 and as far as I can tell there's no reason why
> an aliased variable cannot be marked as replaceable when either the
> alias or the variable it aliases are captured.
>
> Captured simply means that it needs to be wrapped up in the closure,
> AFAIK.  But if it's replaced, then the original variable will need
> to end up in the closure.  The only case where you can't replace is
> if either variable is assigned to, because then they don't point to
> the same thing anymore.

I agree with this.

There's this case where a small improvement could be made. The
replacement is OK if the replacement is only referenced once:

  (let* ((a 1)
 (b a))
(set! b 2)
b)

Except in this case (from compiler-tests.scm):

  (let ((outer-bar (##core#undefined)))
(let ((inner-bar (lambda (x) (if x '1 (outer-bar outer-bar)
(set! outer-bar inner-bar)
(outer-bar '#f)))

This test would catch the above case:

  (or (not assigned)
  (and (not (db-get db name 'global))
   (not captured)
   (let ((refs (db-get db name 'references)))
 (= 1 (length refs)

The added complexity is probably not worth it, though.

###

Regarding the capturing, the capture test is still there:


>(when (and value (not global))
>  (when (eq? '##core#variable (node-class value))
> -  (let* ((name (first (node-parameters value)))
> - (nrefs (db-get db name 'references)) )
> +  (let ((name (first (node-parameters value))) )
>  (when (and (not captured)
  ^^
  Here.
I got the impression you wanted to remove this.

> (or (and (not (db-get db name 'unknown))
>  (db-get db name 'value))
  

This test prevents the replacing of formal lambda parameters:

  (lambda (a)
(let ((b a))
  b))
  -->
  (lambda (a) a)

I don't see a reason for preventing replacement in this case.

This seems to work just fine for the full test:

  (and (not assigned)
   (not (db-get db name 'assigned))
   (or (not (variable-visible? name block-compilation))
   (not (db-get db name 'global

> -   (and (not (db-get db name 'captured))
> -nrefs
> -(= 1 (length nrefs))
> -(not assigned)
> +   (and (not assigned)
>  (not (db-get db name 'assigned))
>  (or (not (variable-visible?
>name block-compilation))

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


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-06-30 Thread Peter Bex
On Sun, Jun 30, 2019 at 08:55:09PM +0200, felix.winkelm...@bevuta.com wrote:
> Yes, this appears to be the correct way - I wondered why there was not
> an additional constraint on assignment, but since the order of optimizations
> is not easily seen in advance, one has to be careful about many assumptions.

Yeah, these things aren't easy.

> I think your reasing is correct, but must ask everybody to let me review this 
> for
> a certain time before we commit this patch.

Thank you for taking the time!  I was hoping you would take a look at
it :)

Note that I forgot to mention in my original mail that I also removed
the constraint that the variable should only be referenced once.  As far
as I can tell, if one instance of a non-assigned variable can be replaced
by the variable it aliases, all other instances should be replaceable in
the same way.

And the first hunk of the patch is not a change in functionality.  It
just changes the comment to reflect reality, but you probably noticed
that already.

Cheers,
Peter


signature.asc
Description: PGP signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-06-30 Thread felix . winkelmann
> I had a look at #1620 and as far as I can tell there's no reason why
> an aliased variable cannot be marked as replaceable when either the
> alias or the variable it aliases are captured.
> 
> Captured simply means that it needs to be wrapped up in the closure,
> AFAIK.  But if it's replaced, then the original variable will need
> to end up in the closure.  The only case where you can't replace is
> if either variable is assigned to, because then they don't point to
> the same thing anymore.
> 
> I've tested several eggs with this and it seems to work fine.  The
> test case in #1620 is now compiled similar to the version with manually
> inserted eq? calls.
> 

Yes, this appears to be the correct way - I wondered why there was not
an additional constraint on assignment, but since the order of optimizations
is not easily seen in advance, one has to be careful about many assumptions.

I think your reasing is correct, but must ask everybody to let me review this 
for
a certain time before we commit this patch.


felix


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


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-06-30 Thread felix . winkelmann
> On Sun, Jun 30, 2019 at 11:33:17AM -0400, John Cowan wrote:
> > Obviously this is not something you can do in a patch,
> > but at some point Chicken may want to go
> > over to the assignment conversion strategy, in which all mutable
> > local variables are transformed into immutable references to boxes.
> 
> I think this already happens in CHICKEN.  When a variable is both
> captured and assigned, it is put in a box (see core.scm:2274).

It does.


felix


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


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-06-30 Thread Peter Bex
On Sun, Jun 30, 2019 at 11:33:17AM -0400, John Cowan wrote:
> Obviously this is not something you can do in a patch,
> but at some point Chicken may want to go
> over to the assignment conversion strategy, in which all mutable
> local variables are transformed into immutable references to boxes.

I think this already happens in CHICKEN.  When a variable is both
captured and assigned, it is put in a box (see core.scm:2274).

I'd be the first to admit I'm not super familiar with this stuff, so
it could be something different from what you're suggesting.

Cheers,
Peter


signature.asc
Description: PGP signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers


Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

2019-06-30 Thread John Cowan
On Sun, Jun 30, 2019 at 9:57 AM Peter Bex  wrote:

Captured simply means that it needs to be wrapped up in the closure,
> AFAIK.  But if it's replaced, then the original variable will need
> to end up in the closure.  The only case where you can't replace is
> if either variable is assigned to, because then they don't point to
> the same thing anymore.
>

Obviously this is not something you can do in a patch,
but at some point Chicken may want to go
over to the assignment conversion strategy, in which all mutable
local variables are transformed into immutable references to boxes.

When the original code uses such a variable, it becomes a
call on box-ref, and a set! of such a variable becomes a call
on box-set!  Eventually the box will be reclaimed by the
garbage collector.  Local mutable variables are rare enough
that this does not create significant memory pressure.

Chez and perhaps other compilers use this transformation because
of how much it simplifies bookkeeping.  In ML, it's actually exposed
to the programmer, and the *only* mutable thing are boxes ("refs")
and mutable arrays.  But of course ML is not Scheme.


John Cowan  http://vrici.lojban.org/~cowanco...@ccil.org
The experiences of the past show that there has always been a discrepancy
between plans and performance.--Emperor Hirohito, August 1945
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers