Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[compiler] Emit Let Bindings Iteratively #14163

Merged
merged 11 commits into from
Jan 22, 2024

Conversation

ehigham
Copy link
Member

@ehigham ehigham commented Jan 16, 2024

Previously Emit(?:Stream)?$ would emit let bindings recursively, regardless of if that binding was used.
If a stream is not used, Emit(?:Stream)?$ would define its missing labels, making emission recursive.
This can lead to stack overflows for large numbers of let-bindings (and does so for the benchmark benchmark matrix-multi-write-nothing).

By not emitting unused streams, we can make let-binding emission iterative.

By the time we get to `Emit(?:Stream)?$`, all bindings of type TStream
should have been:
 - inlined (if it has exactly one use), or
 - eliminated (if it has no uses)

Note streams cannot have more than one use.

Previously `Emit(?:Stream)?$` was written without this assumption and
had to emit stream bindings recursively. This can lead to stack
overflows for large numbers of let-bindings (eg benchmark
`matrix-multi-write-nothing`).

This change emits bindings iteratively with the assertion emit bindings
may not be of type TStream.
@ehigham ehigham marked this pull request as draft January 17, 2024 16:00
@ehigham ehigham marked this pull request as ready for review January 17, 2024 20:34
@ehigham ehigham changed the title [compiler] Flatten Emitting Lets [compiler] Emit Lets Bindings Iteratively Jan 18, 2024
@ehigham ehigham requested a review from chrisvittal January 18, 2024 15:07
@ehigham ehigham changed the title [compiler] Emit Lets Bindings Iteratively [compiler] Emit Let Bindings Iteratively Jan 18, 2024
@ehigham ehigham assigned ehigham and chrisvittal and unassigned ehigham Jan 18, 2024
@danking danking merged commit 8ae336f into hail-is:main Jan 22, 2024
8 checks passed
@ehigham ehigham deleted the ehigham/emit-let-iterative branch January 23, 2024 17:27
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.

3 participants