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

Effects / double translation: keep more functions in direct style #1791

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

vouillon
Copy link
Member

@vouillon vouillon commented Jan 7, 2025

We don't require escaping functions nor functions called from a CPS call site to have a CPS variant. This makes it possible to reduce the amount of code duplication.
This implies that CPS call sites have to be able do deal with direct style functions. This makes CPS calls a little bit slower, but we don't care too much about performance in this case.

- An escaping function does not need to be in CPS
- A CPS call site can call non-CPS functions
@vouillon vouillon force-pushed the improved-double-translation branch from 19ec7d0 to 6b00bf6 Compare January 7, 2025 17:50
@vouillon vouillon marked this pull request as ready for review January 8, 2025 16:00
@vouillon vouillon requested a review from OlivierNicole January 8, 2025 16:56
@hhugo
Copy link
Member

hhugo commented Jan 13, 2025

@vouillon, do you want to merge this before a release ?

@vouillon
Copy link
Member Author

@vouillon, do you want to merge this before a release ?

No, this can be merged later.

@OlivierNicole
Copy link
Contributor

I’m looking at this, but I think I need to reflect a bit more to understand this statement:

We don't require escaping functions nor functions called from a CPS call site to have a CPS variant.

Copy link
Contributor

@OlivierNicole OlivierNicole left a comment

Choose a reason for hiding this comment

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

This looks good to me! I only have a question on an unrelated point.

compiler/lib/effects.ml Show resolved Hide resolved
@OlivierNicole
Copy link
Contributor

OlivierNicole commented Jan 15, 2025

Here’s how this PR affects the (bzip2-ed) code size increase factor induced by --effects=double-translation compared to --effects=cps:

Revision master (3d645b9) this PR
ocamlc (doesn't use effect handlers) 161 % 155 %
generator.ml 111.3 % 111.0 %

On generator.ml which is a small program using effect handlers, the effect is very small.

@OlivierNicole
Copy link
Contributor

If the calling convention can be chosen dynamically, doesn’t it open the way to being able to link together files compiled with --effects=cps and files compiled with --effects=double-translation?

@vouillon
Copy link
Member Author

If the calling convention can be chosen dynamically, doesn’t it open the way to being able to link together files compiled with --effects=cps and files compiled with --effects=double-translation?

With --effects=double-translation , we expect that there will always be a direct style version of all functions, which we can call directly. This is not the case with --effects=cps.

@hhugo hhugo merged commit 830a2ad into master Jan 16, 2025
28 of 29 checks passed
@hhugo hhugo deleted the improved-double-translation branch January 16, 2025 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants