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

[embedded] Introduce class-bound existentials into Embedded Swift #76572

Merged

Conversation

kubamracek
Copy link
Contributor

Motivated by need for protocol-based dynamic dispatch, which hasn't been possible in Embedded Swift due to a full ban on existentials. This lifts that restriction but only for class-bound existentials: Class-bound existentials are already (even in desktop Swift) much more lightweight than full existentials, as they don't need type metadata, their containers are typically 2 words only (reference + wtable pointer), don't incur copies (only retains+releases).

Included in this PR:

  • Non-generic class-bound existentials, executable tests for those.
  • Extension methods on protocols and using those from a class-bound existential.
  • RuntimeEffects now differentiate between Existential and ExistentialClassBound.
  • PerformanceDiagnostics don't flag ExistentialClassBound in Embedded Swift.
  • WTables are generated in IRGen when needed.

Left for follow-up PRs:

  • Generic classes support

Motivated by need for protocol-based dynamic dispatch, which hasn't been possible in Embedded Swift due to a full ban on existentials. This lifts that restriction but only for class-bound existentials: Class-bound existentials are already (even in desktop Swift) much more lightweight than full existentials, as they don't need type metadata, their containers are typically 2 words only (reference + wtable pointer), don't incur copies (only retains+releases).

Included in this PR:
[x] Non-generic class-bound existentials, executable tests for those.
[x] Extension methods on protocols and using those from a class-bound existential.
[x] RuntimeEffects now differentiate between Existential and ExistentialClassBound.
[x] PerformanceDiagnostics don't flag ExistentialClassBound in Embedded Swift.
[x] WTables are generated in IRGen when needed.

Left for follow-up PRs:
[ ] Generic classes support
@@ -963,7 +961,7 @@ RuntimeEffect swift::getRuntimeEffect(SILInstruction *inst, SILType &impactType)
rt |= RuntimeEffect::ObjectiveC | RuntimeEffect::MetaData;
break;
case SILFunctionTypeRepresentation::WitnessMethod:
rt |= RuntimeEffect::MetaData | RuntimeEffect::Existential;
rt |= RuntimeEffect::MetaData; // ???
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eeckstein Do you have a suggestion what to check here? I think we want this to be RuntimeEffect::ExistentialClassBound if the witness method is for a class-bound protocol, and RuntimeEffect::Existential otherwise?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, I think this makes sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, but do you have any hints on how to implement this? We have an ApplySite and an ApplyInst here, how do we go from that to the protocol that's in used?

Copy link
Contributor

Choose a reason for hiding this comment

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

You could do it as in hasValidSignatureForEmbedded: check if all generic parameters are class bound. The function type you can get from ApplySite::getOrigCalleeType()

Copy link
Contributor

Choose a reason for hiding this comment

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

It's even simpler: you can use SILFunctionType::getWitnessMethodConformanceOrInvalid

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, done!

// in embedded Swift mode, running the existential specializer might introduce
// more generic calls from non-generic functions, which breaks the assumptions
// of embedded Swift.
P.addExistentialSpecializer();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eeckstein Any better ideas what to do here? Or is this reasonable to keep? Temporarily?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine

Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

nice!

@@ -963,7 +961,7 @@ RuntimeEffect swift::getRuntimeEffect(SILInstruction *inst, SILType &impactType)
rt |= RuntimeEffect::ObjectiveC | RuntimeEffect::MetaData;
break;
case SILFunctionTypeRepresentation::WitnessMethod:
rt |= RuntimeEffect::MetaData | RuntimeEffect::Existential;
rt |= RuntimeEffect::MetaData; // ???
Copy link
Contributor

Choose a reason for hiding this comment

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

yes, I think this makes sense

// in embedded Swift mode, running the existential specializer might introduce
// more generic calls from non-generic functions, which breaks the assumptions
// of embedded Swift.
P.addExistentialSpecializer();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine

@kubamracek
Copy link
Contributor Author

@swift-ci please test

@kubamracek
Copy link
Contributor Author

@swift-ci please test Linux platform

@kubamracek kubamracek merged commit 4a51cfa into swiftlang:main Sep 20, 2024
4 of 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