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

Lambda-lifting: ensure that functions are move to a more appropriate place #1360

Merged
merged 2 commits into from
Dec 17, 2022

Conversation

vouillon
Copy link
Member

Functions are moved too high by lambda lifting, adding a huge number of parameters. This has a significant impact on code size.

This change ensures that lambda lifting moves function to a more appropriate level, which does not depend on compiling options.

With this change, the code generated for the Ocisgen Start demo is 55% larger (instead of 80% larger). And when compressed with bzip2 this code is only 4% larger (instead of 26% larger).

This is a temporary fix. A more robust solution will be to avoid a CPS transformation of the code at toplevel, which is not necessary since we know that it is not within an effect handler.

@vouillon vouillon mentioned this pull request Dec 16, 2022
@hhugo
Copy link
Member

hhugo commented Dec 16, 2022

Can you explain a bit why the change fixes the issue ?

@hhugo
Copy link
Member

hhugo commented Dec 16, 2022

Maybe we should update the graph in performance.wiki if the size difference is significant ?

@vouillon
Copy link
Member Author

Without this change, a block can be added at the beginning of the code (in particular for a standalone program):

==== 54705 () ====
   v41579 = "caml_fs_init"()
   branch 54704 ()

This branch is turned into a function call to a function containing basically the whole code, which is not inlined since it is so large. With a baseline of 1, functions are lifted outside of this large function.

One could have set a baseline of 2 instead. But with this change, functions are always lifted to the same position.

@vouillon
Copy link
Member Author

Maybe we should update the graph in performance.wiki if the size difference is significant ?

The benchmarks are so small that no lambda lifting is performed.

Maybe I should add something in the documentation about the code size increase for large programs?

@hhugo hhugo mentioned this pull request Dec 16, 2022
3 tasks
@hhugo
Copy link
Member

hhugo commented Dec 16, 2022

Without this change, a block can be added at the beginning of the code (in particular for a standalone program):

==== 54705 () ====
   v41579 = "caml_fs_init"()
   branch 54704 ()

This branch is turned into a function call to a function containing basically the whole code, which is not inlined since it is so large. With a baseline of 1, functions are lifted outside of this large function.

One could have set a baseline of 2 instead. But with this change, functions are always lifted to the same position.

I didn't realize we'were performing cps for every Branch. Couldn't we skip the transformation when there is not call and there is only one edge to the continuation ?

The benchmarks are so small that no lambda lifting is performed.

make -C benchmarks copy-extra-bc should give you bigger programs.

@hhugo hhugo mentioned this pull request Dec 16, 2022
@vouillon
Copy link
Member Author

I didn't realize we'were performing cps for every Branch. Couldn't we skip the transformation when there is not call and there is only one edge to the continuation ?

I'm working on it... Because of inlining, this is not really a problem.

The benchmarks are so small that no lambda lifting is performed.

make -C benchmarks copy-extra-bc should give you bigger programs.

I have updated the manual with these programs.

This ensures that lambda lifting moves function to a more appropriate level.
@hhugo
Copy link
Member

hhugo commented Dec 17, 2022

You haven't updated size-effects.png

@vouillon
Copy link
Member Author

You haven't updated size-effects.png

Indeed! Thanks! This is done, now.

@hhugo hhugo merged commit fbb633e into master Dec 17, 2022
@hhugo hhugo deleted the effect-fix branch December 17, 2022 14:28
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