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

stack_ supports all primitives that returns poly return mode #3471

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

riaqn
Copy link
Contributor

@riaqn riaqn commented Jan 14, 2025

This PR allows writing stack_ (ref "hello").

In general, ref could be any primitive whose return mode is poly. We already implicitly optimize allocation for these case, so stack_ should inherit that behavior.

@riaqn riaqn marked this pull request as draft January 14, 2025 17:59
@goldfirere
Copy link
Collaborator

I see why we might want this, but was this change motivated by a concrete use? In addition, a return mode of poly isn’t always right — for example, %identity has that property, I believe, but isn’t an allocation. If we wanted to do this, I think it would be better just to list out the primitives that optionally stack-allocate. But I probably favor not doing this at all, but instead to make the stack-allocation apparent in the named external, like ref_stack. If someone wants to ensure a stack allocation, they just use ref_stack.

@goldfirere
Copy link
Collaborator

OK — I found the Slack thread that provides more context (and that already suggests not to use the “poly” criterion). But I still think that we should just have a stack_ref instead of allowing this. Let’s continue the design debate on Slack.

@mshinwell mshinwell added the locals Local allocation label Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
locals Local allocation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants