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: assume that works with inlining #1714

Closed

Conversation

gretay-js
Copy link
Contributor

@gretay-js gretay-js commented Aug 9, 2023

This PR propagates [@zero_alloc assume] annotation from a function to its body, piggybacking on Debuginfo.t. The PR starts with a bit of refactoring to make it easier to add a field to Debuginfo.t. I'll be happy to separate the refactoring commit into another PR.

The propagation happens in middle_end during translation from Lambda and boils down to adding a new argument to Debuginfo.from_location and to the "environment" of translation.
This PR a draft because it looks too complicated. The change is separate for each middle end, and I am not sure if I have all the places updated correctly. Is there a better way? Would it be better to add another pass on Lambda dedicated to propagating this information?

Tested on small examples manually with all 3 middle ends. TODO: test on larger examples.

@gretay-js gretay-js force-pushed the zero_alloc_assume_metadata branch from 30bb153 to 16521b8 Compare August 9, 2023 19:11
@gretay-js gretay-js force-pushed the zero_alloc_assume_metadata branch from b0f8ab2 to 95edf88 Compare August 9, 2023 19:32
@gretay-js gretay-js requested a review from lpw25 August 9, 2023 19:34
@gretay-js
Copy link
Contributor Author

I'll split this up and rework the propagation, there are two approaches to try :

  • propagate assume_zero_alloc when the function is inlined, instead of on the function body itself on the way out of lambda.
  • propagate assume_zero_alloc at lambda, instead of doing it in the middle-end.. this means adding this field to Debuginfo.Scoped_location as well.

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.

2 participants