Skip to content

schemify: move let-loop argument binding out of the loop - #5590

Closed
samth wants to merge 1 commit into
racket:masterfrom
samth:schemify-loop-entry
Closed

samth wants to merge 1 commit into
racket:masterfrom
samth:schemify-loop-entry

Conversation

@samth

@samth samth commented Sep 22, 2026

Copy link
Copy Markdown
Member

For loops of the form (let loop ([x e] ...) body), bind the e's outside the letrec when they are simple? so that the extra lets needed to ensure left-to-right evaluation don't interfere with Chez-level loop detection.

Also, fix the implementation of simple? for set! forms.

For loops of the form `(let loop ([x e] ...) body)`, bind the e's
outside the `letrec` when they are `simple?` so that the extra
`let`s needed to ensure left-to-right evaluation don't interfere
with Chez-level loop detection.

Also, fix the implementation of `simple?` for `set!` forms.
@mflatt

mflatt commented Sep 22, 2026

Copy link
Copy Markdown
Member

This looks like a case where it's better to adjust Chez Scheme.

I think you're aiming to cover an example like

(define (f x)
  (let loop ([x (+ x 1)] [y (+ x 2)])
    (if (= x 0)
         'done
        (loop (sub1 x) y))))

where the (+ x 1) and (+ x 2) sequence needs to be ordered, and that turns into a let around the initial call to loop in schemify's output.

I see that the change would cover this case, but with a couple of limitations:

  • It captures the expansion of named let as ((letrec ([loop ....]) loop) arg ...), but not (letrec ([loop ....]) (loop arg ...)), where recognizing the latter would require checking that the args dot no use loop.

  • If (+ x 1) and (+ x 2) are replaced with calls to unknown functions, then the conversion cannot apply. The conversion has to be conservative, because it doesn't readily know whether the loop function is referenced in a non-application position, in which case closure allocation might expose reordering.

The loop-detection pass in Chez Scheme has the information that loop is only applied, and I think it can be less conservative in transformations. Also, that pass recognizes the normalized form where the initial call is inside the letrec body as (loop arg ...).

I have to test more, but I think this commit may be the right idea: mflatt/ChezScheme@f754d7c

@samth

samth commented Sep 22, 2026

Copy link
Copy Markdown
Member Author

That seems like a better improvement. I think the change to simple? should be kept.

If you're looking at loop recognition code, you might also be interested in this commit: samth@e5cabda

That's relatively easy to work around as well by explicitly using let loop, but it did come up in the mandelbrot benchmark we were looking at.

@mflatt

mflatt commented Sep 23, 2026

Copy link
Copy Markdown
Member

If you're looking at loop recognition code, you might also be interested in this commit

It turns out that something like that (generalized) is need to make stable tests, since cp0 may move an application around letrec into the letrec body (I think, but I didn't pin it down specifically).

mflatt added a commit that referenced this pull request Sep 24, 2026
The handling of `set!` was incorrect for non-pure mode (partial
correction in #5590), non-pure mode was too restructive with respect
to mutable variables, and some transformations required a non-pure
simple expression unnecessarily.
@samth

samth commented Sep 25, 2026

Copy link
Copy Markdown
Member Author

Obsoleted by cisco/ChezScheme#1069

@samth samth closed this Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants