Skip to content

SILGen: Don't reference external property descriptors for @backDeployed properties #67982

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
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions lib/SILGen/SILGen.h
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,11 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor<SILGenModule> {
/// Emits a thunk from an actor function to a potentially distributed call.
void emitDistributedThunk(SILDeclRef thunk);

/// Returns true if the given declaration must be referenced through a
/// back deployment thunk in a context with the given resilience expansion.
bool requiresBackDeploymentThunk(ValueDecl *decl,
ResilienceExpansion expansion);

/// Emits a thunk that calls either the original function if it is available
/// or otherwise calls a fallback variant of the function that was emitted
/// into the client module.
Expand Down
33 changes: 4 additions & 29 deletions lib/SILGen/SILGenApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -280,31 +280,6 @@ static void convertOwnershipConventionsGivenParamInfos(
});
}

static bool shouldApplyBackDeploymentThunk(ValueDecl *decl, ASTContext &ctx,
ResilienceExpansion expansion) {
auto backDeployBeforeVersion = decl->getBackDeployedBeforeOSVersion(ctx);
if (!backDeployBeforeVersion)
return false;

// If the context of the application is inlinable then we must always call the
// back deployment thunk since we can't predict the deployment targets of
// other modules.
if (expansion != ResilienceExpansion::Maximal)
return true;

// In resilient function bodies skip calling the back deployment thunk when
// the deployment target is high enough that the ABI implementation of the
// back deployed function is guaranteed to be available.
auto deploymentAvailability = AvailabilityContext::forDeploymentTarget(ctx);
auto declAvailability =
AvailabilityContext(VersionRange::allGTE(*backDeployBeforeVersion));

if (deploymentAvailability.isContainedIn(declAvailability))
return false;

return true;
}

//===----------------------------------------------------------------------===//
// Callee
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -1155,7 +1130,6 @@ class SILGenApply : public Lowering::ExprVisitor<SILGenApply> {

SILDeclRef getDeclRefForStaticDispatchApply(DeclRefExpr *e) {
auto *afd = cast<AbstractFunctionDecl>(e->getDecl());
auto &ctx = SGF.getASTContext();

// A call to a `distributed` function may need to go through a thunk.
if (callSite && callSite->shouldApplyDistributedThunk()) {
Expand All @@ -1164,8 +1138,9 @@ class SILGenApply : public Lowering::ExprVisitor<SILGenApply> {
}

// A call to `@backDeployed` function may need to go through a thunk.
if (shouldApplyBackDeploymentThunk(afd, ctx,
SGF.F.getResilienceExpansion())) {

if (SGF.SGM.requiresBackDeploymentThunk(afd,
SGF.F.getResilienceExpansion())) {
return SILDeclRef(afd).asBackDeploymentKind(
SILDeclRef::BackDeploymentKind::Thunk);
}
Expand Down Expand Up @@ -6717,7 +6692,7 @@ SILDeclRef SILGenModule::getAccessorDeclRef(AccessorDecl *accessor,
ResilienceExpansion expansion) {
auto declRef = SILDeclRef(accessor, SILDeclRef::Kind::Func);

if (shouldApplyBackDeploymentThunk(accessor, getASTContext(), expansion))
if (requiresBackDeploymentThunk(accessor, expansion))
return declRef.asBackDeploymentKind(SILDeclRef::BackDeploymentKind::Thunk);

return declRef.asForeign(requiresForeignEntryPoint(accessor));
Expand Down
30 changes: 30 additions & 0 deletions lib/SILGen/SILGenBackDeploy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,36 @@ static void emitBackDeployForwardApplyAndReturnOrThrow(
SGF.B.createBranch(loc, SGF.ReturnDest.getBlock(), directResults);
}

bool SILGenModule::requiresBackDeploymentThunk(ValueDecl *decl,
ResilienceExpansion expansion) {
auto &ctx = getASTContext();
auto backDeployBeforeVersion = decl->getBackDeployedBeforeOSVersion(ctx);
if (!backDeployBeforeVersion)
return false;

switch (expansion) {
case ResilienceExpansion::Minimal:
// In a minimal resilience expansion we must always call the back deployment
// thunk since we can't predict the deployment targets of the modules that
// might inline the call.
return true;
case ResilienceExpansion::Maximal:
// FIXME: We can skip thunking if we're in the same module.
break;
}

// Use of a back deployment thunk is unnecessary if the deployment target is
// high enough that the ABI implementation of the back deployed declaration is
// guaranteed to be available.
auto deploymentAvailability = AvailabilityContext::forDeploymentTarget(ctx);
auto declAvailability =
AvailabilityContext(VersionRange::allGTE(*backDeployBeforeVersion));
if (deploymentAvailability.isContainedIn(declAvailability))
return false;

return true;
}

void SILGenFunction::emitBackDeploymentThunk(SILDeclRef thunk) {
// Generate code equivalent to:
//
Expand Down
6 changes: 6 additions & 0 deletions lib/SILGen/SILGenExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3821,6 +3821,12 @@ SILGenModule::emitKeyPathComponentForDecl(SILLocation loc,
return false;
}

// Back deployed properties have the same restrictions as
// always-emit-into-client properties.
if (requiresBackDeploymentThunk(baseDecl, expansion)) {
return false;
}

// Properties that only dispatch via ObjC lookup do not have nor
// need property descriptors, since the selector identifies the
// storage.
Expand Down
7 changes: 7 additions & 0 deletions test/SILGen/back_deployed_attr_accessor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,11 @@ func caller(_ s: TopLevelStruct) {
// -- Verify the thunk is called
// CHECK: {{%.*}} = function_ref @$s11back_deploy14TopLevelStructV8propertyACvgTwb : $@convention(method) (TopLevelStruct) -> TopLevelStruct
_ = s.property

// -- Verify key path
// CHECK: {{%.*}} = keypath $KeyPath<TopLevelStruct, TopLevelStruct>, (root $TopLevelStruct; gettable_property $TopLevelStruct, id @$s11back_deploy14TopLevelStructV8propertyACvg : $@convention(method) (TopLevelStruct) -> TopLevelStruct, getter @$s11back_deploy14TopLevelStructV8propertyACvpACTK : $@convention(thin) (@in_guaranteed TopLevelStruct) -> @out TopLevelStruct)
_ = \TopLevelStruct.property
}

// CHECK-LABEL: sil shared [thunk] [ossa] @$s11back_deploy14TopLevelStructV8propertyACvpACTK : $@convention(thin) (@in_guaranteed TopLevelStruct) -> @out TopLevelStruct
// CHECK: function_ref @$s11back_deploy14TopLevelStructV8propertyACvgTwb
2 changes: 2 additions & 0 deletions test/attr/attr_backDeployed_evolution.swift
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ do {

let empty = IntArray.empty
precondition(empty.values == [])
precondition(empty[keyPath: \.values] == [])

var array = IntArray([5])

Expand Down Expand Up @@ -177,6 +178,7 @@ do {
do {
let defaulted = ReferenceIntArray()
precondition(defaulted.values == [])
precondition(defaulted[keyPath: \.values] == [])

let empty = ReferenceIntArray.empty
precondition(empty.values == [])
Expand Down