Skip to content

Conversation

DougGregor
Copy link
Member

When outlining a destroy operation, we form direct calls to the deinit of noncopyable types. For generic types, this was always calling into unspecialized generics, which is... deeply unfortunate.

Look for a specialized deinit and call that instead. This eliminates a compiler assertion in Embedded Swift, and should improve performance with noncopyable generics elsewhere.

Fixes rdar://159054138 and #72627 / rdar://157131184.

@DougGregor DougGregor requested a review from rjmccall as a code owner August 25, 2025 23:07
@DougGregor
Copy link
Member Author

@swift-ci please smoke test

1 similar comment
@DougGregor
Copy link
Member Author

@swift-ci please smoke test

When outlining a destroy operation, we form direct calls to the deinit
of noncopyable types. For generic types, this was always calling into
unspecialized generics, which is... deeply unfortunate.

Look for a specialized deinit and call that instead. This eliminates a
compiler assertion in Embedded Swift, and should improve performance
with noncopyable generics elsewhere.

Fixes rdar://159054138 and swiftlang#72627 / rdar://157131184.
@DougGregor DougGregor force-pushed the specialized-deinit-in-destroy-outlining branch from 2a3a14f to beeb45d Compare August 25, 2025 23:13
@DougGregor
Copy link
Member Author

@swift-ci please smoke test

auto substitutions = ty->getContextSubstitutionMap();
if (!substitutions.empty() &&
!substitutions.getRecursiveProperties().hasArchetype()) {
Mangle::GenericSpecializationMangler mangler(
Copy link
Contributor

Choose a reason for hiding this comment

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

This is kind of gross, is there a better way where we actually store the specializations in a table somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is totally gross, I agree. A better answer I was considering is to allow SILMoveOnlyDeinit to carry a substitution map, and make it possible to look up (nominal type, substitution map) pairs in the module + serialized module. We would specialize the SILMoveOnlyDeinit when we see a nominal type specialization that has a deinit.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to start with my hack to unblock some stuff

@DougGregor DougGregor enabled auto-merge August 26, 2025 04:32
@DougGregor DougGregor merged commit f9394ad into swiftlang:main Aug 26, 2025
3 checks passed
@DougGregor DougGregor deleted the specialized-deinit-in-destroy-outlining branch August 26, 2025 06:19
@eeckstein
Copy link
Contributor

This approach is completely wrong. It just works by coincidence for this specific test case.

The correct fix is: #83913

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.

3 participants