Hi all, At the Sun & Fun conference I had some time to look into #1317, which was tricky to find but once found easy to fix.
The reason the fibc benchmark would hang is because the "addc" procedure gets compiled to a looping C function which builds up stack, and then it needs to perform a minor GC. When it does, it needs to save the argvector so that it can re-invoke the calculation's continuation. The bug is that it would save the _original_ argvector, while the loop will mutate a working copy of each argument. The fix simply refreshes the argvector with the current working values. I decided not to add fibc as a test, because it's such an extremely specific issue, and it's hard to guarantee that it will be compiled exactly in such a way that it triggers this bug. Cheers, Peter
From 7b76d0cbf56d54fc19f16b6aeee5600cb58270b0 Mon Sep 17 00:00:00 2001 From: Peter Bex <[email protected]> Date: Sat, 24 Sep 2016 11:41:41 +0200 Subject: [PATCH] Copy temp arg values back into argvector on loop This would prevent the procedure from starting from scratch after GC. With this change, we continue the calculation, as it should be. Fixes #1317 --- NEWS | 4 ++++ c-backend.scm | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index ebe102c..658e61e 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,9 @@ 4.11.2 +- Compiler: + - Fixed incorrect argvector restoration after GC in directly + recursive functions (#1317). + 4.11.1 - Security fixes diff --git a/c-backend.scm b/c-backend.scm index cbe5895..2479986 100644 --- a/c-backend.scm +++ b/c-backend.scm @@ -845,6 +845,11 @@ (else (gen #\{))))) (cond ((and (not (eq? 'toplevel id)) (not direct)) + (when (and looping (not customizable)) + ;; Loop will update t_n copy of av[n]; refresh av. + (do ((i 0 (add1 i))) + ((>= i n)) + (gen #t "av[" i "]=t" i ";"))) (cond (rest (gen #t "C_save_and_reclaim((void*)" id ",c,av);}" #t "a=C_alloc((c-" n ")*C_SIZEOF_PAIR+" demand ");") @@ -859,7 +864,7 @@ (apply gen arglist) (gen ");}")) (else - (gen "C_save_and_reclaim((void *)" id #\, n ",av);}"))) + (gen #t "C_save_and_reclaim((void *)" id #\, n ",av);}"))) (when (> demand 0) (gen #t "a=C_alloc(" demand ");"))))) (else (gen #\}))) -- 2.1.4
From 1ba194a85da774f3fb563d2b04979bd7dcf4be15 Mon Sep 17 00:00:00 2001 From: Peter Bex <[email protected]> Date: Sat, 24 Sep 2016 11:41:41 +0200 Subject: [PATCH] Copy temp arg values back into argvector on loop This would prevent the procedure from starting from scratch after GC. With this change, we continue the calculation, as it should be. Fixes #1317 --- NEWS | 3 +++ c-backend.scm | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index c7210dd..d27b997 100644 --- a/NEWS +++ b/NEWS @@ -55,6 +55,9 @@ 4.11.2 +- Compiler: + - Fixed incorrect argvector restoration after GC in directly + recursive functions (#1317). 4.11.1 diff --git a/c-backend.scm b/c-backend.scm index e3958c2..c4cd6b3 100644 --- a/c-backend.scm +++ b/c-backend.scm @@ -866,6 +866,11 @@ (else (gen #\{))))) (cond ((and (not (eq? 'toplevel id)) (not direct)) + (when (and looping (not customizable)) + ;; Loop will update t_n copy of av[n]; refresh av. + (do ((i 0 (add1 i))) + ((>= i n)) + (gen #t "av[" i "]=t" i ";"))) (cond (rest (gen #t "C_save_and_reclaim((void*)" id ",c,av);}" #t "a=C_alloc((c-" n ")*C_SIZEOF_PAIR+" demand ");") @@ -880,7 +885,7 @@ (apply gen arglist) (gen ");}")) (else - (gen "C_save_and_reclaim((void *)" id #\, n ",av);}"))) + (gen #t "C_save_and_reclaim((void *)" id #\, n ",av);}"))) (when (> demand 0) (gen #t "a=C_alloc(" demand ");"))))) (else (gen #\}))) -- 2.1.4
signature.asc
Description: Digital signature
_______________________________________________ Chicken-hackers mailing list [email protected] https://lists.nongnu.org/mailman/listinfo/chicken-hackers
