Skip to content

backend: lazy closure restore #4895

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

Draft
wants to merge 5 commits into
base: gabor/compile-tweaks
Choose a base branch
from

Conversation

ggreif
Copy link
Contributor

@ggreif ggreif commented Feb 10, 2025

This gives us a les intrusive way of closure variable restoration. Instead of

# function prelude
  local.get $clos
  i32.load offs_a
  local.set $a
  ...
  local.get $a
  # use $a

one has

# empty prelude
  ...
  local.get $clos
  i32.load offs_a
  # use it instead of $a

Some functions only use closed-over variables to pass them on to other closures! So it makes total sense to avoid pulling them into locals. Such functions arise from (non-inlined) local-async functions.

Note: Right now this change is enabled globally, so I expect that the code size will increase due to closure variables used more than once. When we'll do this selectively, I expect a reduction in code size.

@@ -9125,6 +9125,9 @@ module VarEnv = struct
in the given stackrep (Vanilla, UnboxedWord32, …) so far
Used for immutable and mutable, non-captured data *)
| Local of SR.t * int32
(* An already captured binding from the caller's frame
Used for immutable and mutable, non-captured data *)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Used for immutable and mutable, non-captured data *)
Used for immutable and mutable data *)

@@ -8971,6 +8971,9 @@ module VarEnv = struct
in the given stackrep (Vanilla, UnboxedWord64, …) so far
Used for immutable and mutable, non-captured data *)
| Local of SR.t * int32
(* An already captured binding from the caller's frame
Used for immutable and mutable, non-captured data *)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Used for immutable and mutable, non-captured data *)
Used for immutable and mutable data *)

@ggreif ggreif force-pushed the gabor/lazy-closure-restore branch from a5935df to 47937ad Compare February 11, 2025 06:00
@crusso
Copy link
Contributor

crusso commented Feb 20, 2025

This might also reduce stack pressure - but won't it require repeated forwarding in the incremental gc? @luc-blaeser

@ggreif
Copy link
Contributor Author

ggreif commented Feb 20, 2025

This might also reduce stack pressure - but won't it require repeated forwarding in the incremental gc? @luc-blaeser

Definitely, it's a tradeoff. I expect it to be a total win when we just wrap an inner function (we can detect this in freevars) and use all closure vars linearly. A good example is CPS-converted func() : async () { ... }, where the async block boils down to a function call.

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