Skip to content

SIL: Distinguish "compatible convention" and "compatible representation" function conversions. #28541

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
Dec 4, 2019
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
23 changes: 20 additions & 3 deletions include/swift/SIL/TypeLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -965,10 +965,27 @@ class TypeConverter {
#endif

enum class ABIDifference : uint8_t {
// No ABI differences, function can be trivially bitcast to result type.
Trivial,
// Types have compatible calling conventions and representations, so can
// be trivially bitcast.
CompatibleRepresentation,

// No convention differences, function can be cast via `convert_function`
// without a thunk.
//
// There may still be a representation difference between values of the
// compared function types. This means that, if two function types
// have a matching argument or return of function type with
// `SameCallingConvention`, then the outer function types may not themselves
// have the `SameCallingConvention` because they need a thunk to convert
// the inner function value representation.
CompatibleCallingConvention,
Copy link
Contributor

Choose a reason for hiding this comment

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

The contrast between this and the cases below isn't clear. Maybe something like:

The types have a representation difference that can be resolved with convert_function, such as a possible difference in ptrauth discriminators.

We don't need to be coy about using ptrauth as an example.

The comment about nested positions applies to all these cases except CompatibleRepresentation, so maybe it should just go there.

Also you keep using SameCallingConvention in these comments, but it looks like you renamed that to CompatibleCallingConvention.

Longer-term question: should we just roll thin_to_thick_function into convert_function, so that we have one instruction for doing superficial conversions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'll clean up the comment in a follow up commit.


// Representation difference requires thin-to-thick conversion.
ThinToThick,
CompatibleRepresentation_ThinToThick,
// Function types have the `SameCallingConvention` but additionally need
// a thin-to-thick conversion.
CompatibleCallingConvention_ThinToThick,

// Non-trivial difference requires thunk.
NeedsThunk
};
Expand Down
51 changes: 34 additions & 17 deletions lib/SIL/TypeLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2440,14 +2440,14 @@ TypeConverter::checkForABIDifferences(SILModule &M,
// If the types are identical and there was no optionality change,
// we're done.
if (type1 == type2 && !optionalityChange)
return ABIDifference::Trivial;
return ABIDifference::CompatibleRepresentation;

// Classes, class-constrained archetypes, and pure-ObjC existential types
// all have single retainable pointer representation; optionality change
// is allowed.
if (type1.getASTType()->satisfiesClassConstraint() &&
type2.getASTType()->satisfiesClassConstraint())
return ABIDifference::Trivial;
return ABIDifference::CompatibleRepresentation;

// Function parameters are ABI compatible if their differences are
// trivial.
Expand All @@ -2470,7 +2470,7 @@ TypeConverter::checkForABIDifferences(SILModule &M,
if (meta1->getRepresentation() == meta2->getRepresentation() &&
(!optionalityChange ||
meta1->getRepresentation() == MetatypeRepresentation::Thick))
return ABIDifference::Trivial;
return ABIDifference::CompatibleRepresentation;
}
}

Expand All @@ -2483,7 +2483,7 @@ TypeConverter::checkForABIDifferences(SILModule &M,
if (auto meta2 = type2.getAs<ExistentialMetatypeType>()) {
if (meta1->getRepresentation() == meta2->getRepresentation() &&
meta1->getRepresentation() == MetatypeRepresentation::ObjC)
return ABIDifference::Trivial;
return ABIDifference::CompatibleRepresentation;
}
}

Expand All @@ -2498,12 +2498,12 @@ TypeConverter::checkForABIDifferences(SILModule &M,
if (checkForABIDifferences(M,
type1.getTupleElementType(i),
type2.getTupleElementType(i))
!= ABIDifference::Trivial)
!= ABIDifference::CompatibleRepresentation)
return ABIDifference::NeedsThunk;
}

// Tuple lengths and elements match
return ABIDifference::Trivial;
return ABIDifference::CompatibleRepresentation;
}
}
}
Expand All @@ -2517,10 +2517,19 @@ TypeConverter::ABIDifference
TypeConverter::checkFunctionForABIDifferences(SILModule &M,
SILFunctionType *fnTy1,
SILFunctionType *fnTy2) {
// For now, only differentiate representation from calling convention when
// staging in substituted function types.
//
// We might still want to conditionalize this behavior even after we commit
// substituted function types, to avoid bloating
// IR for platforms that don't differentiate function type representations.
bool DifferentFunctionTypesHaveDifferentRepresentation
= Context.LangOpts.EnableSubstSILFunctionTypesForFunctionValues;

// Fast path -- if both functions were unwrapped from a CanSILFunctionType,
// we might have pointer equality here.
if (fnTy1 == fnTy2)
return ABIDifference::Trivial;
return ABIDifference::CompatibleRepresentation;

if (fnTy1->getParameters().size() != fnTy2->getParameters().size())
return ABIDifference::NeedsThunk;
Expand Down Expand Up @@ -2548,7 +2557,7 @@ TypeConverter::checkFunctionForABIDifferences(SILModule &M,
result1.getSILStorageType(M, fnTy1),
result2.getSILStorageType(M, fnTy2),
/*thunk iuos*/ fnTy1->getLanguage() == SILFunctionLanguage::Swift)
!= ABIDifference::Trivial)
!= ABIDifference::CompatibleRepresentation)
return ABIDifference::NeedsThunk;
}

Expand All @@ -2563,7 +2572,7 @@ TypeConverter::checkFunctionForABIDifferences(SILModule &M,
yield1.getSILStorageType(M, fnTy1),
yield2.getSILStorageType(M, fnTy2),
/*thunk iuos*/ fnTy1->getLanguage() == SILFunctionLanguage::Swift)
!= ABIDifference::Trivial)
!= ABIDifference::CompatibleRepresentation)
return ABIDifference::NeedsThunk;
}

Expand All @@ -2580,7 +2589,7 @@ TypeConverter::checkFunctionForABIDifferences(SILModule &M,
error1.getSILStorageType(M, fnTy1),
error2.getSILStorageType(M, fnTy2),
/*thunk iuos*/ fnTy1->getLanguage() == SILFunctionLanguage::Swift)
!= ABIDifference::Trivial)
!= ABIDifference::CompatibleRepresentation)
return ABIDifference::NeedsThunk;
}

Expand All @@ -2592,26 +2601,34 @@ TypeConverter::checkFunctionForABIDifferences(SILModule &M,

// Parameters are contravariant and our relation is not symmetric, so
// make sure to flip the relation around.
std::swap(param1, param2);

if (checkForABIDifferences(M,
param1.getSILStorageType(M, fnTy1),
param2.getSILStorageType(M, fnTy2),
param1.getSILStorageType(M, fnTy1),
/*thunk iuos*/ fnTy1->getLanguage() == SILFunctionLanguage::Swift)
!= ABIDifference::Trivial)
!= ABIDifference::CompatibleRepresentation)
return ABIDifference::NeedsThunk;
}

auto rep1 = fnTy1->getRepresentation(), rep2 = fnTy2->getRepresentation();
if (rep1 != rep2) {
if (rep1 == SILFunctionTypeRepresentation::Thin &&
rep2 == SILFunctionTypeRepresentation::Thick)
return ABIDifference::ThinToThick;
rep2 == SILFunctionTypeRepresentation::Thick) {
if (DifferentFunctionTypesHaveDifferentRepresentation) {
// FIXME: check whether the representations are compatible modulo
// context
return ABIDifference::CompatibleCallingConvention_ThinToThick;
} else {
return ABIDifference::CompatibleRepresentation_ThinToThick;
}
}

return ABIDifference::NeedsThunk;
}

return ABIDifference::Trivial;
if (DifferentFunctionTypesHaveDifferentRepresentation)
return ABIDifference::CompatibleCallingConvention;
else
return ABIDifference::CompatibleRepresentation;
}

CanSILBoxType
Expand Down
6 changes: 4 additions & 2 deletions lib/SILGen/SILGenExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1463,7 +1463,8 @@ static ManagedValue convertCFunctionSignature(SILGenFunction &SGF,
// ABI-compatible, since we can't emit a thunk.
switch (SGF.SGM.Types.checkForABIDifferences(SGF.SGM.M,
loweredResultTy, loweredDestTy)){
case TypeConverter::ABIDifference::Trivial:
case TypeConverter::ABIDifference::CompatibleRepresentation:
case TypeConverter::ABIDifference::CompatibleCallingConvention:
result = fnEmitter();
assert(result.getType() == loweredResultTy);

Expand All @@ -1482,7 +1483,8 @@ static ManagedValue convertCFunctionSignature(SILGenFunction &SGF,
result = SGF.emitUndef(loweredDestTy);
break;

case TypeConverter::ABIDifference::ThinToThick:
case TypeConverter::ABIDifference::CompatibleCallingConvention_ThinToThick:
case TypeConverter::ABIDifference::CompatibleRepresentation_ThinToThick:
llvm_unreachable("Cannot have thin to thick conversion here");
}

Expand Down
2 changes: 1 addition & 1 deletion lib/SILGen/SILGenPoly.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ ManagedValue Transform::transform(ManagedValue v,
// If the conversion is trivial, just cast.
if (SGF.SGM.Types.checkForABIDifferences(SGF.SGM.M,
v.getType(), loweredResultTy)
== TypeConverter::ABIDifference::Trivial) {
== TypeConverter::ABIDifference::CompatibleRepresentation) {
if (v.getType().isAddress())
return SGF.B.createUncheckedAddrCast(Loc, v, loweredResultTy);
return SGF.B.createUncheckedBitCast(Loc, v, loweredResultTy);
Expand Down
23 changes: 18 additions & 5 deletions lib/SILGen/SILGenType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,24 @@ SILGenModule::emitVTableMethod(ClassDecl *theClass,
// The override member type is semantically a subtype of the base
// member type. If the override is ABI compatible, we do not need
// a thunk.
if (doesNotHaveGenericRequirementDifference && !baseLessVisibleThanDerived &&
M.Types.checkFunctionForABIDifferences(M,
derivedInfo.SILFnType,
overrideInfo.SILFnType) ==
TypeConverter::ABIDifference::Trivial)
bool compatibleCallingConvention;
switch (M.Types.checkFunctionForABIDifferences(M,
derivedInfo.SILFnType,
overrideInfo.SILFnType)) {
case TypeConverter::ABIDifference::CompatibleCallingConvention:
case TypeConverter::ABIDifference::CompatibleRepresentation:
compatibleCallingConvention = true;
break;
case TypeConverter::ABIDifference::NeedsThunk:
compatibleCallingConvention = false;
break;
case TypeConverter::ABIDifference::CompatibleCallingConvention_ThinToThick:
case TypeConverter::ABIDifference::CompatibleRepresentation_ThinToThick:
llvm_unreachable("shouldn't be thick methods");
}
if (doesNotHaveGenericRequirementDifference
&& !baseLessVisibleThanDerived
&& compatibleCallingConvention)
return SILVTable::Entry(base, implFn, implKind);

// Generate the thunk name.
Expand Down
81 changes: 81 additions & 0 deletions test/SILGen/function_type_conversion.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// RUN: %target-swift-frontend -emit-silgen -disable-availability-checking -module-name main -enable-subst-sil-function-types-for-function-values %s | %FileCheck %s

func generic<T, U>(_ f: @escaping (T) -> U) -> (T) -> U { return f }

// CHECK-LABEL: sil {{.*}}4main{{.*}}11sameGeneric
func sameGeneric<X, Y, Z>(_: X, _: Y, _ f: @escaping (Z) -> Z) -> (Z) -> Z {
// CHECK: bb0({{.*}}, [[F:%[0-9]+]] : @guaranteed $@callee_guaranteed <τ_0_0, τ_0_1> in (@in_guaranteed τ_0_0) -> @out τ_0_1 for <Z, Z>
// Similarly generic types should be directly substitutable
// CHECK: [[GENERIC:%.*]] = function_ref @{{.*}}4main{{.*}}7generic
// CHECK: [[RET:%.*]] = apply [[GENERIC]]<Z, Z>([[F]])
// CHECK: return [[RET]]
return generic(f)
}

// CHECK-LABEL: sil {{.*}}4main{{.*}}16concreteIndirect
func concreteIndirect(_ f: @escaping (Any) -> Any) -> (Any) -> Any {
// CHECK: bb0([[F:%[0-9]+]] : @guaranteed $@callee_guaranteed (@in_guaranteed Any) -> @out Any)
// Any is passed indirectly, but is a concrete type, so we need to convert
// to the generic abstraction level
// CHECK: [[F2:%.*]] = copy_value [[F]]
// CHECK: [[GENERIC_F:%.*]] = convert_function [[F2]] : {{.*}} to $@callee_guaranteed <τ_0_0, τ_0_1> in (@in_guaranteed τ_0_0) -> @out τ_0_1 for <Any, Any>
// CHECK: [[GENERIC:%.*]] = function_ref @{{.*}}4main{{.*}}7generic
// CHECK: [[GENERIC_RET:%.*]] = apply [[GENERIC]]<Any, Any>([[GENERIC_F]])
// CHECK: [[RET:%.*]] = convert_function [[GENERIC_RET]] : {{.*}}
// CHECK: return [[RET]]
return generic(f)
}

// CHECK-LABEL: sil {{.*}}4main{{.*}}14concreteDirect
func concreteDirect(_ f: @escaping (Int) -> String) -> (Int) -> String {
// CHECK: bb0([[F:%[0-9]+]] : @guaranteed $@callee_guaranteed (Int) -> @owned String):
// Int and String are passed and returned directly, so we need both
// thunking and conversion to the substituted form
// CHECK: [[F2:%.*]] = copy_value [[F]]
// CHECK: [[REABSTRACT_F:%.*]] = partial_apply {{.*}}([[F2]])
// CHECK: [[GENERIC_F:%.*]] = convert_function [[REABSTRACT_F]] : {{.*}} to $@callee_guaranteed <τ_0_0, τ_0_1> in (@in_guaranteed τ_0_0) -> @out τ_0_1 for <Int, String>
// CHECK: [[GENERIC:%.*]] = function_ref @{{.*}}4main{{.*}}7generic
// CHECK: [[GENERIC_RET:%.*]] = apply [[GENERIC]]<Int, String>([[GENERIC_F]])
// CHECK: [[REABSTRACT_RET:%.*]] = convert_function [[GENERIC_RET]] : {{.*}}
// CHECK: [[RET:%.*]] = partial_apply {{.*}}([[REABSTRACT_RET]])
// CHECK: return [[RET]]

return generic(f)
}

func genericTakesFunction<T, U>(
_ f: @escaping ((T) -> U) -> (T) -> U
) -> ((T) -> U) -> (T) -> U { return f }

func sameGenericTakesFunction<T>(
_ f: @escaping ((T) -> T) -> (T) -> T
) -> ((T) -> T) -> (T) -> T {
return genericTakesFunction(f)
}

// CHECK-LABEL: sil {{.*}}4main29concreteIndirectTakesFunction
func concreteIndirectTakesFunction(
_ f: @escaping ((Any) -> Any) -> (Any) -> Any
) -> ((Any) -> Any) -> (Any) -> Any {
// CHECK: bb0([[F:%[0-9]+]] : @guaranteed $@callee_guaranteed

// Calling convention matches callee, but the representation of the argument
// to `f` needs to change, so we still have to thunk
// CHECK: [[F2:%.*]] = copy_value [[F]]
// CHECK: [[REABSTRACT_F:%.*]] = partial_apply {{.*}}([[F2]])
// CHECK: [[GENERIC_F:%.*]] = convert_function [[REABSTRACT_F]]
// CHECK: [[GENERIC:%.*]] = function_ref @{{.*}}4main{{.*}}20genericTakesFunction
// CHECK: [[GENERIC_RET:%.*]] = apply [[GENERIC]]<Any, Any>([[GENERIC_F]])
// CHECK: [[REABSTRACT_RET:%.*]] = convert_function [[GENERIC_RET]] : {{.*}}
// CHECK: [[RET:%.*]] = partial_apply {{.*}}([[REABSTRACT_RET]])
// CHECK: return [[RET]]
return genericTakesFunction(f)
}

func concreteDirectTakesFunction(
_ f: @escaping ((Int) -> String) -> (Int) -> String
) -> ((Int) -> String) -> (Int) -> String {
// Int and String are passed and returned directly, so we need both
// thunking and conversion to the substituted form
return genericTakesFunction(f)
}