Skip to content

Commit 549a45e

Browse files
author
Gabor Horvath
committed
Revert "Revert "[SILGen] Fix the type of closure thunks that are passed const reference structs (#76903)" (#77309)"
This reverts commit f73c2e5.
1 parent 1e96466 commit 549a45e

File tree

13 files changed

+189
-34
lines changed

13 files changed

+189
-34
lines changed

‎include/swift/SIL/SILBridging.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -952,7 +952,7 @@ struct BridgedSuccessorArray {
952952
};
953953

954954
struct BridgedDeclRef {
955-
uint64_t storage[3];
955+
uint64_t storage[4];
956956

957957
BRIDGED_INLINE BridgedDeclRef(swift::SILDeclRef declRef);
958958
BRIDGED_INLINE swift::SILDeclRef unbridged() const;
@@ -965,7 +965,7 @@ struct BridgedDeclRef {
965965
};
966966

967967
struct BridgedVTableEntry {
968-
uint64_t storage[5];
968+
uint64_t storage[6];
969969

970970
enum class Kind {
971971
Normal,
@@ -1033,7 +1033,7 @@ struct BridgedConstExprFunctionState {
10331033
};
10341034

10351035
struct BridgedWitnessTableEntry {
1036-
uint64_t storage[5];
1036+
uint64_t storage[6];
10371037

10381038
enum class Kind {
10391039
invalid,

‎include/swift/SIL/SILDeclRef.h‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ namespace llvm {
3434
class raw_ostream;
3535
}
3636

37+
namespace clang {
38+
class Type;
39+
}
40+
3741
namespace swift {
3842
enum class EffectsKind : uint8_t;
3943
class AbstractFunctionDecl;
@@ -208,6 +212,9 @@ struct SILDeclRef {
208212
const GenericSignatureImpl *, CustomAttr *>
209213
pointer;
210214

215+
// Type of closure thunk.
216+
const clang::Type *thunkType = nullptr;
217+
211218
/// Returns the type of AST node location being stored by the SILDeclRef.
212219
LocKind getLocKind() const {
213220
if (loc.is<ValueDecl *>())
@@ -261,11 +268,10 @@ struct SILDeclRef {
261268
/// for the containing ClassDecl.
262269
/// - If 'loc' is a global VarDecl, this returns its GlobalAccessor
263270
/// SILDeclRef.
264-
explicit SILDeclRef(
265-
Loc loc,
266-
bool isForeign = false,
267-
bool isDistributed = false,
268-
bool isDistributedLocal = false);
271+
explicit SILDeclRef(Loc loc, bool isForeign = false,
272+
bool isDistributed = false,
273+
bool isDistributedLocal = false,
274+
const clang::Type *thunkType = nullptr);
269275

270276
/// See above put produces a prespecialization according to the signature.
271277
explicit SILDeclRef(Loc loc, GenericSignature prespecializationSig);

‎lib/SIL/IR/SILDeclRef.cpp‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,15 @@ SILDeclRef::SILDeclRef(ValueDecl *vd, SILDeclRef::Kind kind, bool isForeign,
135135
isAsyncLetClosure(0), pointer(derivativeId) {}
136136

137137
SILDeclRef::SILDeclRef(SILDeclRef::Loc baseLoc, bool asForeign,
138-
bool asDistributed, bool asDistributedKnownToBeLocal)
138+
bool asDistributed, bool asDistributedKnownToBeLocal,
139+
const clang::Type *thunkType)
139140
: isRuntimeAccessible(false),
140141
backDeploymentKind(SILDeclRef::BackDeploymentKind::None),
141142
defaultArgIndex(0), isAsyncLetClosure(0),
142-
pointer((AutoDiffDerivativeFunctionIdentifier *)nullptr) {
143+
pointer((AutoDiffDerivativeFunctionIdentifier *)nullptr),
144+
thunkType(thunkType) {
145+
assert((!thunkType || baseLoc.is<AbstractClosureExpr *>()) &&
146+
"thunk type is needed only for closures");
143147
if (auto *vd = baseLoc.dyn_cast<ValueDecl*>()) {
144148
if (auto *fd = dyn_cast<FuncDecl>(vd)) {
145149
// Map FuncDecls directly to Func SILDeclRefs.
@@ -169,6 +173,8 @@ SILDeclRef::SILDeclRef(SILDeclRef::Loc baseLoc, bool asForeign,
169173
llvm_unreachable("invalid loc decl for SILDeclRef!");
170174
}
171175
} else if (auto *ACE = baseLoc.dyn_cast<AbstractClosureExpr *>()) {
176+
assert((!asForeign || thunkType) &&
177+
"thunk type needed for foreign type for closures");
172178
loc = ACE;
173179
kind = Kind::Func;
174180
if (ACE->getASTContext().LangOpts.hasFeature(

‎lib/SIL/IR/SILFunctionType.cpp‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4286,12 +4286,10 @@ static CanSILFunctionType getUncachedSILFunctionTypeForConstant(
42864286
// The type of the native-to-foreign thunk for a swift closure.
42874287
if (constant.isForeign && constant.hasClosureExpr() &&
42884288
shouldStoreClangType(TC.getDeclRefRepresentation(constant))) {
4289-
auto clangType = TC.Context.getClangFunctionType(
4290-
origLoweredInterfaceType->getParams(),
4291-
origLoweredInterfaceType->getResult(),
4292-
FunctionTypeRepresentation::CFunctionPointer);
4293-
AbstractionPattern pattern =
4294-
AbstractionPattern(origLoweredInterfaceType, clangType);
4289+
assert(!extInfoBuilder.getClangTypeInfo().empty() &&
4290+
"clang type not found");
4291+
AbstractionPattern pattern = AbstractionPattern(
4292+
origLoweredInterfaceType, extInfoBuilder.getClangTypeInfo().getType());
42954293
return getSILFunctionTypeForAbstractCFunction(
42964294
TC, pattern, origLoweredInterfaceType, extInfoBuilder, constant);
42974295
}
@@ -4799,9 +4797,13 @@ getAbstractionPatternForConstant(ASTContext &ctx, SILDeclRef constant,
47994797
if (!constant.isForeign)
48004798
return AbstractionPattern(fnType);
48014799

4800+
if (constant.thunkType)
4801+
return AbstractionPattern(fnType, constant.thunkType);
4802+
48024803
auto bridgedFn = getBridgedFunction(constant);
48034804
if (!bridgedFn)
48044805
return AbstractionPattern(fnType);
4806+
48054807
const clang::Decl *clangDecl = bridgedFn->getClangDecl();
48064808
if (!clangDecl)
48074809
return AbstractionPattern(fnType);

‎lib/SILGen/SILGenBridging.cpp‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,8 +1315,9 @@ static SILValue emitObjCUnconsumedArgument(SILGenFunction &SGF,
13151315
SILLocation loc,
13161316
SILValue arg) {
13171317
auto &lowering = SGF.getTypeLowering(arg->getType());
1318-
// If address-only, make a +1 copy and operate on that.
1319-
if (lowering.isAddressOnly() && SGF.useLoweredAddresses()) {
1318+
// If arg is non-trivial and has an address type, make a +1 copy and operate
1319+
// on that.
1320+
if (!lowering.isTrivial() && arg->getType().isAddress()) {
13201321
auto tmp = SGF.emitTemporaryAllocation(loc, arg->getType().getObjectType());
13211322
SGF.B.createCopyAddr(loc, arg, tmp, IsNotTake, IsInitialization);
13221323
return tmp;
@@ -1448,6 +1449,11 @@ emitObjCThunkArguments(SILGenFunction &SGF, SILLocation loc, SILDeclRef thunk,
14481449
auto buf = SGF.emitTemporaryAllocation(loc, native.getType());
14491450
native.forwardInto(SGF, loc, buf);
14501451
native = SGF.emitManagedBufferWithCleanup(buf);
1452+
} else if (!fnConv.isSILIndirect(nativeInputs[i]) &&
1453+
native.getType().isAddress()) {
1454+
// Load the value if the argument has an address type and the native
1455+
// function expects the argument to be passed directly.
1456+
native = SGF.emitManagedLoadCopy(loc, native.getValue());
14511457
}
14521458

14531459
if (nativeInputs[i].isConsumedInCaller()) {

‎lib/SILGen/SILGenExpr.cpp‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1735,7 +1735,19 @@ static ManagedValue convertCFunctionSignature(SILGenFunction &SGF,
17351735
FunctionConversionExpr *e,
17361736
SILType loweredResultTy,
17371737
llvm::function_ref<ManagedValue ()> fnEmitter) {
1738-
SILType loweredDestTy = SGF.getLoweredType(e->getType());
1738+
SILType loweredDestTy;
1739+
auto destTy = e->getType();
1740+
auto clangInfo =
1741+
destTy->castTo<AnyFunctionType>()->getExtInfo().getClangTypeInfo();
1742+
if (clangInfo.empty())
1743+
loweredDestTy = SGF.getLoweredType(destTy);
1744+
else
1745+
// This won't be necessary after we stop dropping clang types when
1746+
// canonicalizing function types.
1747+
loweredDestTy = SGF.getLoweredType(
1748+
AbstractionPattern(destTy->getCanonicalType(), clangInfo.getType()),
1749+
destTy);
1750+
17391751
ManagedValue result;
17401752

17411753
// We're converting between C function pointer types. They better be
@@ -1806,7 +1818,9 @@ ManagedValue emitCFunctionPointer(SILGenFunction &SGF,
18061818
#endif
18071819
semanticExpr = conv->getSubExpr()->getSemanticsProvidingExpr();
18081820
}
1809-
1821+
1822+
const clang::Type *destFnType = nullptr;
1823+
18101824
if (auto declRef = dyn_cast<DeclRefExpr>(semanticExpr)) {
18111825
setLocFromConcreteDeclRef(declRef->getDeclRef());
18121826
} else if (auto memberRef = dyn_cast<MemberRefExpr>(semanticExpr)) {
@@ -1820,12 +1834,18 @@ ManagedValue emitCFunctionPointer(SILGenFunction &SGF,
18201834
loc = closure;
18211835
return ManagedValue();
18221836
});
1837+
auto clangInfo = conversionExpr->getType()
1838+
->castTo<FunctionType>()
1839+
->getExtInfo()
1840+
.getClangTypeInfo();
1841+
if (!clangInfo.empty())
1842+
destFnType = clangInfo.getType();
18231843
} else {
18241844
llvm_unreachable("c function pointer converted from a non-concrete decl ref");
18251845
}
18261846

18271847
// Produce a reference to the C-compatible entry point for the function.
1828-
SILDeclRef constant(loc, /*foreign*/ true);
1848+
SILDeclRef constant(loc, /*foreign*/ true, false, false, destFnType);
18291849
SILConstantInfo constantInfo =
18301850
SGF.getConstantInfo(SGF.getTypeExpansionContext(), constant);
18311851

‎lib/Serialization/Deserialization.cpp‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8446,9 +8446,6 @@ class SwiftToClangBasicReader :
84468446

84478447
llvm::Expected<const clang::Type *>
84488448
ModuleFile::getClangType(ClangTypeID TID) {
8449-
if (!getContext().LangOpts.UseClangFunctionTypes)
8450-
return nullptr;
8451-
84528449
if (TID == 0)
84538450
return nullptr;
84548451

‎lib/Serialization/Serialization.cpp‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5874,10 +5874,7 @@ class Serializer::TypeSerializer : public TypeVisitor<TypeSerializer> {
58745874
using namespace decls_block;
58755875

58765876
auto resultType = S.addTypeRef(fnTy->getResult());
5877-
auto clangType =
5878-
S.getASTContext().LangOpts.UseClangFunctionTypes
5879-
? S.addClangTypeRef(fnTy->getClangTypeInfo().getType())
5880-
: ClangTypeID(0);
5877+
auto clangType = S.addClangTypeRef(fnTy->getClangTypeInfo().getType());
58815878

58825879
auto isolation = encodeIsolation(fnTy->getIsolation());
58835880

‎test/Interop/Cxx/class/Inputs/closure.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ struct NonTrivial {
1010
int *p;
1111
};
1212

13+
struct Trivial {
14+
int i;
15+
};
16+
1317
void cfunc(void (^ _Nonnull block)(NonTrivial)) noexcept {
1418
block(NonTrivial());
1519
}
@@ -45,4 +49,13 @@ void cfuncARCWeak(ARCWeak) noexcept;
4549
void (* _Nonnull getFnPtr() noexcept)(NonTrivial) noexcept;
4650
void (* _Nonnull getFnPtr2() noexcept)(ARCWeak) noexcept;
4751

52+
void cfuncConstRefNonTrivial(void (*_Nonnull)(const NonTrivial &));
53+
void cfuncConstRefTrivial(void (*_Nonnull)(const Trivial &));
54+
void blockConstRefNonTrivial(void (^_Nonnull)(const NonTrivial &));
55+
void blockConstRefTrivial(void (^_Nonnull)(const Trivial &));
56+
#if __OBJC__
57+
void cfuncConstRefStrong(void (*_Nonnull)(const ARCStrong &));
58+
void blockConstRefStrong(void (^_Nonnull)(const ARCStrong &));
59+
#endif
60+
4861
#endif // __CLOSURE__

‎test/Interop/Cxx/class/closure-thunk-macosx.swift‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,88 @@ public func testClosureToFuncPtr() {
3535
public func testClosureToBlockReturnNonTrivial() {
3636
cfuncReturnNonTrivial({() -> NonTrivial in return NonTrivial() })
3737
}
38+
39+
// CHECK-LABEL: sil private [thunk] [ossa] @$s4main22testConstRefNonTrivialyyFySo0eF0VcfU_To : $@convention(c) (@in_guaranteed NonTrivial) -> () {
40+
// CHECK: bb0(%[[V0:.*]] : $*NonTrivial):
41+
// CHECK: %[[V1:.*]] = alloc_stack $NonTrivial
42+
// CHECK: copy_addr %[[V0]] to [init] %[[V1]] : $*NonTrivial
43+
// CHECK: %[[V3:.*]] = function_ref @$s4main22testConstRefNonTrivialyyFySo0eF0VcfU_ : $@convention(thin) (@in_guaranteed NonTrivial) -> ()
44+
// CHECK: %[[V4:.*]] = apply %[[V3]](%[[V1]]) : $@convention(thin) (@in_guaranteed NonTrivial) -> ()
45+
// CHECK: destroy_addr %[[V1]] : $*NonTrivial
46+
// CHECK: dealloc_stack %[[V1]] : $*NonTrivial
47+
// CHECK: return %[[V4]] : $()
48+
49+
public func testConstRefNonTrivial() {
50+
cfuncConstRefNonTrivial({S in });
51+
}
52+
53+
// CHECK-LABEL: sil private [thunk] [ossa] @$s4main19testConstRefTrivialyyFySo0E0VcfU_To : $@convention(c) (@in_guaranteed Trivial) -> () {
54+
// CHECK: bb0(%[[V0:.*]] : $*Trivial):
55+
// CHECK: %[[V1:.*]] = load [trivial] %[[V0]] : $*Trivial
56+
// CHECK: %[[V2:.*]] = function_ref @$s4main19testConstRefTrivialyyFySo0E0VcfU_ : $@convention(thin) (Trivial) -> ()
57+
// CHECK: %[[V3:.*]] = apply %[[V2]](%[[V1]]) : $@convention(thin) (Trivial) -> ()
58+
// CHECK: return %[[V3]] : $()
59+
60+
public func testConstRefTrivial() {
61+
cfuncConstRefTrivial({S in });
62+
}
63+
64+
// CHECK-LABEL: sil private [thunk] [ossa] @$s4main18testConstRefStrongyyFySo9ARCStrongVcfU_To : $@convention(c) (@in_guaranteed ARCStrong) -> () {
65+
// CHECK: bb0(%[[V0:.*]] : $*ARCStrong):
66+
// CHECK: %[[V1:.*]] = alloc_stack $ARCStrong
67+
// CHECK: copy_addr %[[V0]] to [init] %[[V1]] : $*ARCStrong
68+
// CHECK: %[[V3:.*]] = load [copy] %[[V1]] : $*ARCStrong
69+
// CHECK: %[[V4:.*]] = begin_borrow %[[V3]] : $ARCStrong
70+
// CHECK: %[[V5:.*]] = function_ref @$s4main18testConstRefStrongyyFySo9ARCStrongVcfU_ : $@convention(thin) (@guaranteed ARCStrong) -> ()
71+
// CHECK: %[[V6:.*]] = apply %[[V5]](%[[V4]]) : $@convention(thin) (@guaranteed ARCStrong) -> ()
72+
// CHECK: end_borrow %[[V4]] : $ARCStrong
73+
// CHECK: destroy_value %[[V3]] : $ARCStrong
74+
// CHECK: destroy_addr %[[V1]] : $*ARCStrong
75+
// CHECK: dealloc_stack %[[V1]] : $*ARCStrong
76+
// CHECK: return %[[V6]] : $()
77+
78+
public func testConstRefStrong() {
79+
cfuncConstRefStrong({S in });
80+
}
81+
82+
// CHECK-LABEL: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @$sSo10NonTrivialVIegn_ABIeyBn_TR : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@in_guaranteed NonTrivial) -> (), @in_guaranteed NonTrivial) -> () {
83+
// CHECK: bb0(%[[V0:.*]] : $*@block_storage @callee_guaranteed (@in_guaranteed NonTrivial) -> (), %[[V1:.*]] : $*NonTrivial):
84+
// CHECK: %[[V2:.*]] = project_block_storage %[[V0]] : $*@block_storage @callee_guaranteed (@in_guaranteed NonTrivial) -> ()
85+
// CHECK: %[[V3:.*]] = load [copy] %[[V2]] : $*@callee_guaranteed (@in_guaranteed NonTrivial) -> ()
86+
// CHECK: %[[V4:.*]] = begin_borrow %[[V3]] : $@callee_guaranteed (@in_guaranteed NonTrivial) -> ()
87+
// CHECK: apply %[[V4]](%[[V1]]) : $@callee_guaranteed (@in_guaranteed NonTrivial) -> ()
88+
// CHECK: end_borrow %[[V4]] : $@callee_guaranteed (@in_guaranteed NonTrivial) -> ()
89+
// CHECK: destroy_value %[[V3]] : $@callee_guaranteed (@in_guaranteed NonTrivial) -> ()
90+
91+
public func testBlockConstRefNonTrivial() {
92+
blockConstRefNonTrivial({S in });
93+
}
94+
95+
// CHECK-LABEL: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @$sSo7TrivialVIegy_ABIeyBn_TR : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (Trivial) -> (), @in_guaranteed Trivial) -> () {
96+
// CHECK: bb0(%[[V0:.*]] : $*@block_storage @callee_guaranteed (Trivial) -> (), %[[V1:.*]] : $*Trivial):
97+
// CHECK: %[[V2:.*]] = project_block_storage %[[V0]] : $*@block_storage @callee_guaranteed (Trivial) -> ()
98+
// CHECK: %[[V3:.*]] = load [copy] %[[V2]] : $*@callee_guaranteed (Trivial) -> ()
99+
// CHECK: %[[V4:.*]] = load [trivial] %[[V1]] : $*Trivial
100+
// CHECK: %[[V5:.*]] = begin_borrow %[[V3]] : $@callee_guaranteed (Trivial) -> ()
101+
// CHECK: apply %[[V5]](%[[V4]]) : $@callee_guaranteed (Trivial) -> ()
102+
// CHECK: end_borrow %[[V5]] : $@callee_guaranteed (Trivial) -> ()
103+
// CHECK: destroy_value %[[V3]] : $@callee_guaranteed (Trivial) -> ()
104+
105+
public func testBlockConstRefTrivial() {
106+
blockConstRefTrivial({S in });
107+
}
108+
109+
// CHECK-LABEL: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @$sSo9ARCStrongVIegg_ABIeyBn_TR : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@guaranteed ARCStrong) -> (), @in_guaranteed ARCStrong) -> () {
110+
// CHECK: bb0(%[[V0:.*]] : $*@block_storage @callee_guaranteed (@guaranteed ARCStrong) -> (), %[[V1:.*]] : $*ARCStrong):
111+
// CHECK: %[[V2:.*]] = project_block_storage %[[V0]] : $*@block_storage @callee_guaranteed (@guaranteed ARCStrong) -> ()
112+
// CHECK: %[[V3:.*]] = load [copy] %[[V2]] : $*@callee_guaranteed (@guaranteed ARCStrong) -> ()
113+
// CHECK: %[[V4:.*]] = load_borrow %[[V1]] : $*ARCStrong
114+
// CHECK: %[[V5:.*]] = begin_borrow %[[V3]] : $@callee_guaranteed (@guaranteed ARCStrong) -> ()
115+
// CHECK: apply %[[V5]](%[[V4]]) : $@callee_guaranteed (@guaranteed ARCStrong) -> ()
116+
// CHECK: end_borrow %[[V5]] : $@callee_guaranteed (@guaranteed ARCStrong) -> ()
117+
// CHECK: end_borrow %[[V4]] : $ARCStrong
118+
// CHECK: destroy_value %[[V3]] : $@callee_guaranteed (@guaranteed ARCStrong) -> ()
119+
120+
public func testBlockConstRefStrong() {
121+
blockConstRefStrong({S in });
122+
}

0 commit comments

Comments
 (0)