Skip to content

OpaqueArchetypeSpecializer: inlinable functions are now in this module #27856

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.

Sign up for GitHub

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 3 commits into from
Oct 24, 2019
Merged

OpaqueArchetypeSpecializer: inlinable functions are now in this module #27856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b08014a
Make OpaqueSubstitutionKind an enum class. No functionality change.
jrose-apple Oct 23, 2019
a7f6e3d
OpaqueArchetypeSpecializer: inlinable functions are now in this module
jrose-apple Oct 24, 2019
6c1afdc
Constify ReplaceOpaqueTypesWithUnderlyingTypes's DeclContext
jrose-apple Oct 24, 2019
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
8 changes: 4 additions & 4 deletions include/swift/AST/Types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4876,7 +4876,7 @@ class OpaqueTypeArchetypeType final : public ArchetypeType,
BEGIN_CAN_TYPE_WRAPPER(OpaqueTypeArchetypeType, ArchetypeType)
END_CAN_TYPE_WRAPPER(OpaqueTypeArchetypeType, ArchetypeType)

enum OpaqueSubstitutionKind {
enum class OpaqueSubstitutionKind {
// Don't substitute the opaque type for the underlying type.
DontSubstitute,
// Substitute without looking at the type and context.
Expand All @@ -4885,7 +4885,7 @@ enum OpaqueSubstitutionKind {
AlwaysSubstitute,
// Substitute in the same module into a maximal resilient context.
// Can be done if the underlying type is accessible from the context we
// substitute into. Private types cannot be accesses from a different TU.
// substitute into. Private types cannot be accessed from a different TU.
SubstituteSameModuleMaximalResilience,
// Substitute in a different module from the opaque definining decl. Can only
// be done if the underlying type is public.
Expand All @@ -4898,9 +4898,9 @@ enum OpaqueSubstitutionKind {
/// to their underlying types.
class ReplaceOpaqueTypesWithUnderlyingTypes {
public:
DeclContext *inContext;
const DeclContext *inContext;
ResilienceExpansion contextExpansion;
ReplaceOpaqueTypesWithUnderlyingTypes(DeclContext *inContext,
ReplaceOpaqueTypesWithUnderlyingTypes(const DeclContext *inContext,
ResilienceExpansion contextExpansion)
: inContext(inContext), contextExpansion(contextExpansion) {}

Expand Down
22 changes: 14 additions & 8 deletions lib/AST/Type.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2647,34 +2647,40 @@ ReplaceOpaqueTypesWithUnderlyingTypes::shouldPerformSubstitution(
}

static Type
substOpaqueTypesWithUnderlyingTypes(Type ty, DeclContext *inContext,
substOpaqueTypesWithUnderlyingTypes(Type ty, const DeclContext *inContext,
ResilienceExpansion contextExpansion) {
ReplaceOpaqueTypesWithUnderlyingTypes replacer(inContext, contextExpansion);
return ty.subst(replacer, replacer, SubstFlags::SubstituteOpaqueArchetypes);
}

bool canSubstituteTypeInto(Type ty, DeclContext *dc,
OpaqueSubstitutionKind kind) {
/// Checks that \p dc has access to \p ty for the purposes of an opaque
/// substitution described by \p kind.
///
/// This is purely an implementation detail check about whether type metadata
/// will be accessible. It's not intended to enforce any rules about what
/// opaque substitutions are or are not allowed.
static bool canSubstituteTypeInto(Type ty, const DeclContext *dc,
OpaqueSubstitutionKind kind) {
auto nominal = ty->getAnyNominal();
if (!nominal)
return true;

switch (kind) {
case DontSubstitute:
case OpaqueSubstitutionKind::DontSubstitute:
return false;

case AlwaysSubstitute:
case OpaqueSubstitutionKind::AlwaysSubstitute:
return true;

case SubstituteSameModuleMaximalResilience:
case OpaqueSubstitutionKind::SubstituteSameModuleMaximalResilience:
// In the same file any visibility is okay.
if (!dc->isModuleContext() &&
nominal->getDeclContext()->getParentSourceFile() ==
dc->getParentSourceFile())
return true;
return nominal->getEffectiveAccess() > AccessLevel::FilePrivate;

case SubstituteNonResilientModule:
case OpaqueSubstitutionKind::SubstituteNonResilientModule:
// Can't access types that are not public from a different module.
return nominal->getEffectiveAccess() > AccessLevel::Internal;
}
Expand Down Expand Up @@ -2729,7 +2735,7 @@ operator()(SubstitutableType *maybeOpaqueType) const {

static ProtocolConformanceRef
substOpaqueTypesWithUnderlyingTypes(ProtocolConformanceRef ref, Type origType,
DeclContext *inContext,
const DeclContext *inContext,
ResilienceExpansion contextExpansion) {
ReplaceOpaqueTypesWithUnderlyingTypes replacer(inContext, contextExpansion);
return ref.subst(origType, replacer, replacer,
Expand Down
5 changes: 3 additions & 2 deletions lib/SIL/SILFunctionType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2805,8 +2805,9 @@ static bool areABICompatibleParamsOrReturns(SILType a, SILType b,
if (inFunction) {
auto opaqueTypesSubsituted = aa;
auto *dc = inFunction->getDeclContext();
if (!dc)
dc = inFunction->getModule().getSwiftModule();
auto *currentModule = inFunction->getModule().getSwiftModule();
if (!dc || !dc->isChildContextOf(currentModule))
dc = currentModule;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it not be correct to always use currentModule here? I think the SILFunction's dc is only meant for debugging.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it would be correct. The intention of using the current functions current decl context is to be able to optimize the case when you have private declarations in the same sourc file.

Along the way of this micro optimization I introduced the bug that Jordan is fixing here :( .

ReplaceOpaqueTypesWithUnderlyingTypes replacer(
dc, inFunction->getResilienceExpansion());
if (aa.getASTType()->hasOpaqueArchetype())
Expand Down
33 changes: 16 additions & 17 deletions lib/SILOptimizer/Transforms/SpecializeOpaqueArchetypes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,26 @@ llvm::cl::opt<bool>

using namespace swift;

static const DeclContext *
getDeclContextIfInCurrentModule(const SILFunction &fn) {
auto *dc = fn.getDeclContext();
auto *currentModule = fn.getModule().getSwiftModule();
if (dc && dc->isChildContextOf(currentModule))
return dc;
return currentModule;
}

static Type substOpaqueTypesWithUnderlyingTypes(
Type ty, SILFunction *context) {
auto *dc = context->getDeclContext();
if (!dc)
dc = context->getModule().getSwiftModule();

auto *dc = getDeclContextIfInCurrentModule(*context);
ReplaceOpaqueTypesWithUnderlyingTypes replacer(
dc, context->getResilienceExpansion());
return ty.subst(replacer, replacer, SubstFlags::SubstituteOpaqueArchetypes);
}

static SubstitutionMap
substOpaqueTypesWithUnderlyingTypes(SubstitutionMap map, SILFunction *context) {
auto *dc = context->getDeclContext();
if (!dc)
dc = context->getModule().getSwiftModule();
auto *dc = getDeclContextIfInCurrentModule(*context);
ReplaceOpaqueTypesWithUnderlyingTypes replacer(
dc, context->getResilienceExpansion());
return map.subst(replacer, replacer, SubstFlags::SubstituteOpaqueArchetypes);
Expand Down Expand Up @@ -331,9 +335,7 @@ class OpaqueSpecializerCloner
SILType &Sty = TypeCache[Ty];
if (Sty)
return Sty;
auto *dc = Original.getDeclContext();
if (!dc)
dc = Original.getModule().getSwiftModule();
auto *dc = getDeclContextIfInCurrentModule(Original);

// Apply the opaque types substitution.
ReplaceOpaqueTypesWithUnderlyingTypes replacer(
Expand All @@ -351,9 +353,7 @@ class OpaqueSpecializerCloner

ProtocolConformanceRef remapConformance(Type type,
ProtocolConformanceRef conf) {
auto *dc = Original.getDeclContext();
if (!dc)
dc = Original.getModule().getSwiftModule();
auto *dc = getDeclContextIfInCurrentModule(Original);
// Apply the opaque types substitution.
ReplaceOpaqueTypesWithUnderlyingTypes replacer(
dc, Original.getResilienceExpansion());
Expand Down Expand Up @@ -504,14 +504,13 @@ class OpaqueArchetypeSpecializer : public SILFunctionTransform {

return ty.findIf([=](Type type) -> bool {
if (auto opaqueTy = type->getAs<OpaqueTypeArchetypeType>()) {
auto *dc = context->getDeclContext();
if (!dc)
dc = context->getModule().getSwiftModule();
auto opaque = opaqueTy->getDecl();
return ReplaceOpaqueTypesWithUnderlyingTypes::
OpaqueSubstitutionKind subKind =
ReplaceOpaqueTypesWithUnderlyingTypes::
shouldPerformSubstitution(opaque,
context->getModule().getSwiftModule(),
context->getResilienceExpansion());
return subKind != OpaqueSubstitutionKind::DontSubstitute;
}
return false;
});
Expand Down
16 changes: 16 additions & 0 deletions test/SILOptimizer/Inputs/specialize_opaque_type_archetypes_3.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,19 @@ public struct ResilientContainer {
print(x)
}
}

public struct WrapperP2<Wrapped: ExternalP2>: ExternalP2 {
public init(_ wrapped: Wrapped) {}
public func myValue3() -> Int64 { 0 }
}

public func externalResilientWrapper<Wrapped: ExternalP2>(_ wrapped: Wrapped) -> some ExternalP2 {
return WrapperP2(wrapped)
}

@inlinable
@inline(never)
public func inlinableExternalResilientWrapper<Wrapped: ExternalP2>(_ wrapped: Wrapped) -> some ExternalP2 {
return WrapperP2(wrapped)
}

43 changes: 41 additions & 2 deletions test/SILOptimizer/specialize_opaque_type_archetypes.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
// RUN: %target-swift-frontend -disable-availability-checking %S/Inputs/specialize_opaque_type_archetypes_4.swift -I %t -enable-library-evolution -module-name External3 -emit-module -emit-module-path %t/External3.swiftmodule
// RUN: %target-swift-frontend -disable-availability-checking %S/Inputs/specialize_opaque_type_archetypes_3.swift -I %t -enable-library-evolution -module-name External2 -Osize -emit-module -o - | %target-sil-opt -module-name External2 | %FileCheck --check-prefix=RESILIENT %s
// RUN: %target-swift-frontend -disable-availability-checking -I %t -module-name A -enforce-exclusivity=checked -Osize -emit-sil -sil-verify-all %s | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-ptrsize
// RUN: %target-swift-frontend -disable-availability-checking -I %t -module-name A -enforce-exclusivity=checked -Osize -emit-sil -sil-verify-all %s | %FileCheck %s
// RUN: %target-swift-frontend -disable-availability-checking -I %t -module-name A -enforce-exclusivity=checked -enable-library-evolution -Osize -emit-sil -sil-verify-all %s | %FileCheck %s
// RUN: %target-swift-frontend -disable-availability-checking -I %t -module-name A -enforce-exclusivity=checked -enable-library-evolution -Osize -emit-sil -sil-verify-all %s | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-ptrsize

import External
import External2
Expand Down Expand Up @@ -546,3 +545,43 @@ func useAbstractFunction<T: P>(_ fn: (Int64) -> T) {}
public func testThinToThick() {
useAbstractFunction(bar)
}

// CHECK-LABEL: sil @$s1A19rdar56410009_normalyyF
public func rdar56410009_normal() {
// CHECK: [[EXTERNAL_RESILIENT_WRAPPER:%.+]] = function_ref @$s9External224externalResilientWrapperyQrxAA10ExternalP2RzlF
// CHECK: = apply [[EXTERNAL_RESILIENT_WRAPPER]]<@_opaqueReturnTypeOf("$s9External217externalResilientQryF", 0) 🦸>({{%.+}}, {{%.+}}) : $@convention(thin) <τ_0_0 where τ_0_0 : ExternalP2> (@in_guaranteed τ_0_0) -> @out @_opaqueReturnTypeOf("$s9External224externalResilientWrapperyQrxAA10ExternalP2RzlF", 0) 🦸<τ_0_0>
_ = externalResilientWrapper(externalResilient())
} // CHECK: end sil function '$s1A19rdar56410009_normalyyF'

// CHECK-LABEL: sil @$s1A25rdar56410009_inlinedInneryyF
public func rdar56410009_inlinedInner() {
// CHECK: [[EXTERNAL_RESILIENT_WRAPPER:%.+]] = function_ref @$s9External224externalResilientWrapperyQrxAA10ExternalP2RzlF
// CHECK: = apply [[EXTERNAL_RESILIENT_WRAPPER]]<Int64>({{%.+}}, {{%.+}}) : $@convention(thin) <τ_0_0 where τ_0_0 : ExternalP2> (@in_guaranteed τ_0_0) -> @out @_opaqueReturnTypeOf("$s9External224externalResilientWrapperyQrxAA10ExternalP2RzlF", 0) 🦸<τ_0_0>
_ = externalResilientWrapper(inlinableExternalResilient())
} // CHECK: end sil function '$s1A25rdar56410009_inlinedInneryyF'

// CHECK-LABEL: sil @$s1A25rdar56410009_inlinedOuteryyF
public func rdar56410009_inlinedOuter() {
// CHECK: [[INLINABLE_EXTERNAL_RESILIENT_WRAPPER:%.+]] = function_ref @$s9External233inlinableExternalResilientWrapperyQrxAA0C2P2RzlFAA08externalD0QryFQOyQo__Tg5
// CHECK: = apply [[INLINABLE_EXTERNAL_RESILIENT_WRAPPER]]({{%.+}}, {{%.+}}) : $@convention(thin) (@in_guaranteed @_opaqueReturnTypeOf("$s9External217externalResilientQryF", 0) 🦸) -> @out @_opaqueReturnTypeOf("$s9External233inlinableExternalResilientWrapperyQrxAA0C2P2RzlF", 0) 🦸<@_opaqueReturnTypeOf("$s9External217externalResilientQryF", 0) 🦸>
_ = inlinableExternalResilientWrapper(externalResilient())
} // CHECK: end sil function '$s1A25rdar56410009_inlinedOuteryyF'

// Specialized from above
// CHECK-LABEL: sil shared [noinline] @$s9External233inlinableExternalResilientWrapperyQrxAA0C2P2RzlFAA08externalD0QryFQOyQo__Tg5
// CHECK: [[WRAPPER_INIT:%.+]] = function_ref @$s9External29WrapperP2VyACyxGxcfC
// CHECK: = apply [[WRAPPER_INIT]]<@_opaqueReturnTypeOf("$s9External217externalResilientQryF", 0) 🦸>({{%.+}}, {{%.+}}, {{%.+}}) : $@convention(method) <τ_0_0 where τ_0_0 : ExternalP2> (@in τ_0_0, @thin WrapperP2<τ_0_0>.Type) -> @out WrapperP2<τ_0_0>
// CHECK: end sil function '$s9External233inlinableExternalResilientWrapperyQrxAA0C2P2RzlFAA08externalD0QryFQOyQo__Tg5'

// Specialized from below
// CHECK-LABEL: sil shared [noinline] @$s9External233inlinableExternalResilientWrapperyQrxAA0C2P2RzlFs5Int64V_Tg5
// CHECK: [[WRAPPER_INIT:%.+]] = function_ref @$s9External29WrapperP2VyACyxGxcfC
// CHECK: = apply [[WRAPPER_INIT]]<Int64>({{%.+}}, {{%.+}}, {{%.+}}) : $@convention(method) <τ_0_0 where τ_0_0 : ExternalP2> (@in τ_0_0, @thin WrapperP2<τ_0_0>.Type) -> @out WrapperP2<τ_0_0>
// CHECK: end sil function '$s9External233inlinableExternalResilientWrapperyQrxAA0C2P2RzlFs5Int64V_Tg5'

// CHECK-LABEL: sil @$s1A24rdar56410009_inlinedBothyyF
public func rdar56410009_inlinedBoth() {
// CHECK: [[INLINABLE_EXTERNAL_RESILIENT_WRAPPER:%.+]] = function_ref @$s9External233inlinableExternalResilientWrapperyQrxAA0C2P2RzlFs5Int64V_Tg5
// CHECK: = apply [[INLINABLE_EXTERNAL_RESILIENT_WRAPPER]]({{%.+}}, {{%.+}}) : $@convention(thin) (Int64) -> @out @_opaqueReturnTypeOf("$s9External233inlinableExternalResilientWrapperyQrxAA0C2P2RzlF", 0) 🦸<Int64>
_ = inlinableExternalResilientWrapper(inlinableExternalResilient())
} // CHECK: end sil function '$s1A24rdar56410009_inlinedBothyyF'