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

Zero alloc: propagate assume on partial applications #2543

Closed

Conversation

gretay-js
Copy link
Contributor

This PR proposes to propagate "assume" annotation from a partial application to application to extra arguments.

This came up in a discussion of #2165 (see example below).
It came up again recently in #2459 where a new test has different analysis results depending on compiler configuration with or without stack allocation, even though the generated code is the same and there is no stack allocation.

The fix is easy but I am not sure it's the right thing to do.

In the examples below, the generated code for functions g0-g3 is the same.
The check pass on g0 but fails on g1 below, as expected.
Should the check fail or pass on g2 and g3?

let[@inline never][@local never] f x y = (x,y)

let[@zero_alloc] g0 () =
  let x = (f[@zero_alloc assume]) () () in x

let[@zero_alloc] g1 () =
  let x = ((f[@zero_alloc assume]) ()) () in x

let[@zero_alloc] g2 () =
  let x = ((f ())[@zero_alloc assume]) () in x

let[@zero_alloc] g3 () =
  let x = (((f[@zero_alloc assume]) ())[@zero_alloc assume]) () in x

This PR updates the tests for g2 and g3 to pass and removes a special case for heap allocation in a test recenlty added by #2459.

@gretay-js gretay-js requested a review from ccasin May 7, 2024 14:21
@gretay-js gretay-js added the zero alloc zero-alloc check label May 7, 2024
@gretay-js
Copy link
Contributor Author

Propagating assume in g2 is unsound because the user didn't say that the first application is zero_alloc, so we shouldn't merge this PR. We can try to address the problem observed in #2459 in some other way. It seems to be a special case of primitives that return Lapply and it seems to only happen for apply and revapply.

@gretay-js gretay-js closed this May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
zero alloc zero-alloc check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant