Skip to content

SIL: Lower lifetime dependencies when lowering function types. #79351

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

Merged
merged 1 commit into from
Feb 17, 2025

Conversation

jckarter
Copy link
Contributor

Map the lifetime dependencies described in terms of the formal AST-level parameters to the correct parameter(s) in the lowered SIL function type. There can be 0, 1, or many SIL parameters per formal parameter because of tuple exploding. Also, record which dependencies are on addressable parameters (meaning that the dependency includes not only the value of the parameter, but its specific memory location).

@jckarter
Copy link
Contributor Author

@swift-ci Please test

Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

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

Thanks! That was more work than I expected. I think we eventually end up wanting ABI-affecting properties of the function signature to be lowered, so it's good to have it there.

Also, to preserve semantics we wouldn't want to specialize the convention to be direct (although that's not relevant for ABI)

I actually expected addressable parameters to be part of the SIL function independent of lifetime dependencies though. Are you sure we'll never need want that?

@jckarter
Copy link
Contributor Author

My thinking was that lifetime dependencies are ultimately what force the materialization of the addressable parameter and what dictate its lifetime. To me it seemed useful to track it as part of the dependency, since if the dependent parameter or return value could be eliminated by optimization, that would also naturally remove the barrier to eliminating the memory location when nothing is dependent on it anymore.

We do have the ad-hoc use of @_addressable for C++ imports, but nontrivial C++ types are inherently address-only and non-reabstractable, and it seems like their C++-ness should already be a barrier to passes inserting copies that weren't already there. If it's useful, I could also add it as a SIL parameter attribute; would it still be interesting to track as dependency information in that case?

@jckarter
Copy link
Contributor Author

@swift-ci Please test

// memory location in addition to the value stored at that location.
if (P.isInSILMode()
&& name.str() == "address"
&& P.Tok.is(tok::integer_literal)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I glossed over this in my initial "review". I don't think the parser should have any logic that ties addressable parameters to lifetime descriptors. A lifetime descriptor is an element of the syntax. It doesn't tell you whether a lifetime dependency exists.

Copy link
Contributor Author

@jckarter jckarter Feb 14, 2025

Choose a reason for hiding this comment

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

This is purely there to allow the LifetimeDependenceInfo to be parsed back from textual SIL. The LifetimeDependenceInfo still contains the canonical info in the end. This form isn't allowed in Swift code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh thanks. This isSILMode is a pretty big clue there

Map the lifetime dependencies described in terms of the formal AST-level parameters
to the correct parameter(s) in the lowered SIL function type. There can be 0, 1,
or many SIL parameters per formal parameter because of tuple exploding. Also,
record which dependencies are on addressable parameters (meaning that the dependency
includes not only the value of the parameter, but its specific memory location).
@jckarter
Copy link
Contributor Author

@swift-ci Please test

@jckarter jckarter merged commit f778170 into swiftlang:main Feb 17, 2025
5 checks passed
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