Skip to content

Commit 0b6fd1f

Browse files
meg-guptameg-gupta
committed
Use in_guaranteed for let captures
With this all let values will be captured with in_guaranteed convention by the closure. Following are the main changes : SILGen changes: - A new CaptureKind::Immutable is introduced, to capture let values as in_guaranteed. - SILGen of in_guaranteed capture had to be fixed. in_guaranteed captures as per convention are consumed by the closure. And so SILGen should not generate a destroy_addr for an in_guaranteed capture. But LetValueInitialization can push Dealloc and Release states of the captured arg in the Cleanup stack, and there is no way to access the CleanupHandle and disable the emission of destroy_addr while emitting the captures in SILGenFunction::emitCaptures. So we now create, temporary allocation of the in_guaranteed capture iduring SILGenFunction::emitCaptures without emitting destroy_addr for it. SILOptimizer changes: - Handle in_guaranteed in CopyForwarding. - Adjust dealloc_stack of in_guaranteed capture to occur after destroy_addr for on_stack closures in ClosureLifetimeFixup. IRGen changes : - Since HeapLayout can be non-fixed now, make sure emitSize is used conditionally - Don't consider ClassPointerSource kind parameter type for fulfillments while generating code for partial apply forwarder. The TypeMetadata of ClassPointSource kind sources are not populated in HeapLayout's NecessaryBindings. If we have a generic parameter on the HeapLayout which can be fulfilled by a ClassPointerSource, its TypeMetaData will not be found while constructing the dtor function of the HeapLayout. So it is important to skip considering sources of ClassPointerSource kind, so that TypeMetadata of a dependent generic parameters gets populated in HeapLayout's NecessaryBindings.
1 parent 261e9e4 commit 0b6fd1f

18 files changed

+161
-69
lines changed

‎include/swift/SIL/TypeLowering.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,8 @@ enum class CaptureKind {
561561
StorageAddress,
562562
/// A local value captured as a constant.
563563
Constant,
564+
/// A let constant captured as a pointer to storage
565+
Immutable
564566
};
565567

566568

‎include/swift/SILOptimizer/Utils/InstOptUtils.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,12 +344,19 @@ void releasePartialApplyCapturedArg(
344344
SILParameterInfo paramInfo,
345345
InstModCallbacks callbacks = InstModCallbacks());
346346

347+
void deallocPartialApplyCapturedArg(
348+
SILBuilder &builder, SILLocation loc, SILValue arg,
349+
SILParameterInfo paramInfo);
350+
347351
/// Insert destroys of captured arguments of partial_apply [stack].
348352
void insertDestroyOfCapturedArguments(
349353
PartialApplyInst *pai, SILBuilder &builder,
350354
llvm::function_ref<bool(SILValue)> shouldInsertDestroy =
351355
[](SILValue arg) -> bool { return true; });
352356

357+
void insertDeallocOfCapturedArguments(
358+
PartialApplyInst *pai, SILBuilder &builder);
359+
353360
/// This iterator 'looks through' one level of builtin expect users exposing all
354361
/// users of the looked through builtin expect instruction i.e it presents a
355362
/// view that shows all users as if there were no builtin expect instructions

‎lib/IRGen/GenFunc.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1511,7 +1511,7 @@ Optional<StackAddress> irgen::emitFunctionPartialApplication(
15111511
HeapNonFixedOffsets offsets(IGF, layout);
15121512
if (outType->isNoEscape()) {
15131513
stackAddr = IGF.emitDynamicAlloca(
1514-
IGF.IGM.Int8Ty, layout.emitSize(IGF.IGM), Alignment(16));
1514+
IGF.IGM.Int8Ty, layout.isFixedLayout() ? layout.emitSize(IGF.IGM) : offsets.getSize() , Alignment(16));
15151515
stackAddr = stackAddr->withAddress(IGF.Builder.CreateBitCast(
15161516
stackAddr->getAddress(), IGF.IGM.OpaquePtrTy));
15171517
data = stackAddr->getAddress().getAddress();

‎lib/IRGen/GenProto.cpp

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ class PolymorphicConvention {
100100

101101
FulfillmentMap Fulfillments;
102102

103+
bool ForPartialApplyForwarder;
104+
103105
GenericSignature::ConformsToArray getConformsTo(Type t) {
104106
return Generics->getConformsTo(t);
105107
}
@@ -111,7 +113,7 @@ class PolymorphicConvention {
111113
}
112114

113115
public:
114-
PolymorphicConvention(IRGenModule &IGM, CanSILFunctionType fnType);
116+
PolymorphicConvention(IRGenModule &IGM, CanSILFunctionType fnType, bool forPartialApplyForwarder);
115117

116118
ArrayRef<MetadataSource> getSources() const { return Sources; }
117119

@@ -178,8 +180,10 @@ class PolymorphicConvention {
178180
} // end anonymous namespace
179181

180182
PolymorphicConvention::PolymorphicConvention(IRGenModule &IGM,
181-
CanSILFunctionType fnType)
182-
: IGM(IGM), M(*IGM.getSwiftModule()), FnType(fnType) {
183+
CanSILFunctionType fnType,
184+
bool forPartialApplyForwarder = false)
185+
: IGM(IGM), M(*IGM.getSwiftModule()), FnType(fnType),
186+
ForPartialApplyForwarder(forPartialApplyForwarder) {
183187
initGenerics();
184188

185189
auto rep = fnType->getRepresentation();
@@ -217,11 +221,11 @@ PolymorphicConvention::PolymorphicConvention(IRGenModule &IGM,
217221
considerParameter(params[selfIndex], selfIndex, true);
218222
}
219223

220-
// Now consider the rest of the parameters.
221-
for (auto index : indices(params)) {
222-
if (index != selfIndex)
223-
considerParameter(params[index], index, false);
224-
}
224+
// Now consider the rest of the parameters.
225+
for (auto index : indices(params)) {
226+
if (index != selfIndex)
227+
considerParameter(params[index], index, false);
228+
}
225229
}
226230
}
227231

@@ -384,6 +388,10 @@ void PolymorphicConvention::considerParameter(SILParameterInfo param,
384388
case ParameterConvention::Direct_Owned:
385389
case ParameterConvention::Direct_Unowned:
386390
case ParameterConvention::Direct_Guaranteed:
391+
// Don't consider ClassPointer source for partial_apply forwader.
392+
// If not, we may end up with missing TypeMetadata for a type dependent generic parameter
393+
// while generating code for destructor of HeapLayout.
394+
if (ForPartialApplyForwarder) return;
387395
// Classes are sources of metadata.
388396
if (type->getClassOrBoundGenericClass()) {
389397
considerNewTypeSource(MetadataSource::Kind::ClassPointer,
@@ -3000,7 +3008,7 @@ NecessaryBindings NecessaryBindings::computeBindings(
30003008
return bindings;
30013009

30023010
// Figure out what we're actually required to pass:
3003-
PolymorphicConvention convention(IGM, origType);
3011+
PolymorphicConvention convention(IGM, origType, forPartialApplyForwarder);
30043012

30053013
// - unfulfilled requirements
30063014
convention.enumerateUnfulfilledRequirements(

‎lib/SIL/SILFunctionType.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1437,9 +1437,8 @@ lowerCaptureContextParameters(TypeConverter &TC, SILDeclRef function,
14371437
case CaptureKind::Constant: {
14381438
// Constants are captured by value.
14391439
ParameterConvention convention;
1440-
if (loweredTL.isAddressOnly()) {
1441-
convention = ParameterConvention::Indirect_In_Guaranteed;
1442-
} else if (loweredTL.isTrivial()) {
1440+
assert (!loweredTL.isAddressOnly());
1441+
if (loweredTL.isTrivial()) {
14431442
convention = ParameterConvention::Direct_Unowned;
14441443
} else {
14451444
convention = ParameterConvention::Direct_Guaranteed;
@@ -1472,6 +1471,16 @@ lowerCaptureContextParameters(TypeConverter &TC, SILDeclRef function,
14721471
inputs.push_back(param);
14731472
break;
14741473
}
1474+
case CaptureKind::Immutable: {
1475+
// 'let' constants that are address-only are captured as the address of the value
1476+
// and will be consumed by the closure.
1477+
SILType ty = loweredTy.getAddressType();
1478+
auto param =
1479+
SILParameterInfo(ty.getASTType(),
1480+
ParameterConvention::Indirect_In_Guaranteed);
1481+
inputs.push_back(param);
1482+
break;
1483+
}
14751484
}
14761485
}
14771486
}

‎lib/SIL/TypeLowering.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,11 @@ CaptureKind TypeConverter::getDeclCaptureKind(CapturedValue capture,
125125
return CaptureKind::Box;
126126
}
127127

128+
// For 'let' constants
129+
if (!var->supportsMutation()) {
130+
return CaptureKind::Immutable;
131+
}
132+
128133
// If we're capturing into a non-escaping closure, we can generally just
129134
// capture the address of the value as no-escape.
130135
return (capture.isNoEscape()

‎lib/SILGen/SILGenFunction.cpp

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,7 @@ void SILGenFunction::emitCaptures(SILLocation loc,
262262
case CaptureKind::Constant:
263263
capturedArgs.push_back(emitUndef(getLoweredType(type)));
264264
break;
265+
case CaptureKind::Immutable:
265266
case CaptureKind::StorageAddress:
266267
capturedArgs.push_back(emitUndef(getLoweredType(type).getAddressType()));
267268
break;
@@ -333,7 +334,23 @@ void SILGenFunction::emitCaptures(SILLocation loc,
333334
capturedArgs.push_back(emitManagedRValueWithCleanup(Val));
334335
break;
335336
}
336-
337+
case CaptureKind::Immutable: {
338+
if (canGuarantee) {
339+
auto entryValue = getAddressValue(Entry.value);
340+
// No-escaping stored declarations are captured as the
341+
// address of the value.
342+
assert(entryValue->getType().isAddress() && "no address for captured var!");
343+
capturedArgs.push_back(ManagedValue::forLValue(entryValue));
344+
}
345+
else {
346+
auto entryValue = getAddressValue(Entry.value);
347+
auto addr = emitTemporaryAllocation(vd, entryValue->getType());
348+
auto val = B.createCopyAddr(loc, entryValue, addr, IsNotTake,
349+
IsInitialization);
350+
capturedArgs.push_back(ManagedValue::forLValue(addr));
351+
}
352+
break;
353+
}
337354
case CaptureKind::StorageAddress: {
338355
auto entryValue = getAddressValue(Entry.value);
339356
// No-escaping stored declarations are captured as the

‎lib/SILGen/SILGenProlog.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,7 @@ static void emitCaptureArguments(SILGenFunction &SGF,
420420
SGF.B.createDebugValueAddr(Loc, addr, DbgVar);
421421
break;
422422
}
423+
case CaptureKind::Immutable:
423424
case CaptureKind::StorageAddress: {
424425
// Non-escaping stored decls are captured as the address of the value.
425426
auto type = getVarTypeInCaptureContext();

‎lib/SILOptimizer/Mandatory/ClosureLifetimeFixup.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,27 @@ static bool tryRewriteToPartialApplyStack(
459459
saveDeleteInst(convertOrPartialApply);
460460
saveDeleteInst(origPA);
461461

462+
ApplySite site(newPA);
463+
SILFunctionConventions calleeConv(site.getSubstCalleeType(),
464+
newPA->getModule());
465+
466+
// Since we create temporary allocation for in_guaranteed captures during SILGen,
467+
// the dealloc_stack of it can occur before the apply due to conversion scopes.
468+
// When we insert destroy_addr of the in_guaranteed capture after the apply,
469+
// we may end up with a situation when the dealloc_stack occurs before the destroy_addr.
470+
// The code below proactively removes the dealloc_stack of in_guaranteed capture,
471+
// so that it can be reinserted at the correct place after the destroy_addr below.
472+
for (auto &arg : newPA->getArgumentOperands()) {
473+
unsigned calleeArgumentIndex = site.getCalleeArgIndex(arg);
474+
assert(calleeArgumentIndex >= calleeConv.getSILArgIndexOfFirstParam());
475+
auto paramInfo = calleeConv.getParamInfoForSILArg(calleeArgumentIndex);
476+
if (paramInfo.getConvention() == ParameterConvention::Indirect_In_Guaranteed)
477+
// go over all the dealloc_stack, remove it
478+
for (auto *use : arg.get()->getUses())
479+
if (auto *deallocInst = dyn_cast<DeallocStackInst>(use->getUser()))
480+
deallocInst->eraseFromParent();
481+
}
482+
462483
// Insert destroys of arguments after the apply and the dealloc_stack.
463484
if (auto *apply = dyn_cast<ApplyInst>(singleApplyUser)) {
464485
auto insertPt = std::next(SILBasicBlock::iterator(apply));
@@ -468,11 +489,15 @@ static bool tryRewriteToPartialApplyStack(
468489
SILBuilderWithScope b3(insertPt);
469490
b3.createDeallocStack(loc, newPA);
470491
insertDestroyOfCapturedArguments(newPA, b3);
492+
// dealloc_stack of the in_guaranteed capture is inserted
493+
insertDeallocOfCapturedArguments(newPA, b3);
471494
} else if (auto *tai = dyn_cast<TryApplyInst>(singleApplyUser)) {
472495
for (auto *succBB : tai->getSuccessorBlocks()) {
473496
SILBuilderWithScope b3(succBB->begin());
474497
b3.createDeallocStack(loc, newPA);
475498
insertDestroyOfCapturedArguments(newPA, b3);
499+
// dealloc_stack of the in_guaranteed capture is inserted
500+
insertDeallocOfCapturedArguments(newPA, b3);
476501
}
477502
} else {
478503
llvm_unreachable("Unknown FullApplySite instruction kind");

‎lib/SILOptimizer/Transforms/TempRValueElimination.cpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,23 @@ bool TempRValueOptPass::collectLoads(
119119
LLVM_DEBUG(llvm::dbgs()
120120
<< " Temp use may write/destroy its source" << *user);
121121
return false;
122-
123122
case SILInstructionKind::BeginAccessInst:
124123
return cast<BeginAccessInst>(user)->getAccessKind() == SILAccessKind::Read;
125124

125+
case SILInstructionKind::MarkDependenceInst: {
126+
auto mdi = cast<MarkDependenceInst>(user);
127+
if (mdi->getBase() == address) {
128+
return true;
129+
}
130+
for (auto *mdiUseOper : mdi->getUses()) {
131+
if (!collectLoads(mdiUseOper, mdiUseOper->getUser(), mdi, srcObject,
132+
loadInsts))
133+
return false;
134+
}
135+
return true;
136+
}
126137
case SILInstructionKind::ApplyInst:
138+
case SILInstructionKind::PartialApplyInst:
127139
case SILInstructionKind::TryApplyInst: {
128140
ApplySite apply(user);
129141

‎lib/SILOptimizer/Utils/InstOptUtils.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1372,6 +1372,15 @@ void swift::releasePartialApplyCapturedArg(SILBuilder &builder, SILLocation loc,
13721372
callbacks.createdNewInst(u.get<ReleaseValueInst *>());
13731373
}
13741374

1375+
void swift::deallocPartialApplyCapturedArg(SILBuilder &builder, SILLocation loc,
1376+
SILValue arg,
1377+
SILParameterInfo paramInfo) {
1378+
if (!paramInfo.isIndirectInGuaranteed())
1379+
return;
1380+
1381+
builder.createDeallocStack(loc, arg);
1382+
}
1383+
13751384
static bool
13761385
deadMarkDependenceUser(SILInstruction *inst,
13771386
SmallVectorImpl<SILInstruction *> &deleteInsts) {
@@ -1937,6 +1946,22 @@ void swift::insertDestroyOfCapturedArguments(
19371946
}
19381947
}
19391948

1949+
void swift::insertDeallocOfCapturedArguments(
1950+
PartialApplyInst *pai, SILBuilder &builder) {
1951+
assert(pai->isOnStack());
1952+
1953+
ApplySite site(pai);
1954+
SILFunctionConventions calleeConv(site.getSubstCalleeType(),
1955+
pai->getModule());
1956+
auto loc = RegularLocation::getAutoGeneratedLocation();
1957+
for (auto &arg : pai->getArgumentOperands()) {
1958+
unsigned calleeArgumentIndex = site.getCalleeArgIndex(arg);
1959+
assert(calleeArgumentIndex >= calleeConv.getSILArgIndexOfFirstParam());
1960+
auto paramInfo = calleeConv.getParamInfoForSILArg(calleeArgumentIndex);
1961+
deallocPartialApplyCapturedArg(builder, loc, arg.get(), paramInfo);
1962+
}
1963+
}
1964+
19401965
AbstractFunctionDecl *swift::getBaseMethod(AbstractFunctionDecl *FD) {
19411966
while (FD->getOverriddenDecl()) {
19421967
FD = FD->getOverriddenDecl();

‎test/IRGen/closure.swift

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,19 @@ func b<T : Ordinable>(seq seq: T) -> (Int) -> Int {
3131
// CHECK: }
3232

3333
// -- Closure entry point
34-
// CHECK: define internal swiftcc i64 @"$s7closure1b3seqS2icx_tAA9OrdinableRzlFS2icfU_"(i64 %0, %swift.refcounted* %1, %swift.type* %T, i8** %T.Ordinable) {{.*}} {
34+
// CHECK: define internal swiftcc i64 @"$s7closure1b3seqS2icx_tAA9OrdinableRzlFS2icfU_"(i64 %0, %swift.opaque* noalias nocapture %1, %swift.type* %T, i8** %T.Ordinable) {{.*}} {
3535

3636
// -- partial_apply stub
3737
// CHECK: define internal swiftcc i64 @"$s7closure1b3seqS2icx_tAA9OrdinableRzlFS2icfU_TA"(i64 %0, %swift.refcounted* swiftself %1) {{.*}} {
3838
// CHECK: entry:
39-
// CHECK: [[CONTEXT:%.*]] = bitcast %swift.refcounted* %1 to <{ %swift.refcounted, [16 x i8], %swift.refcounted* }>*
40-
// CHECK: [[BINDINGSADDR:%.*]] = getelementptr inbounds <{ %swift.refcounted, [16 x i8], %swift.refcounted* }>, <{ %swift.refcounted, [16 x i8], %swift.refcounted* }>* [[CONTEXT]], i32 0, i32 1
39+
// CHECK: [[CONTEXT:%.*]] = bitcast %swift.refcounted* %1 to <{ %swift.refcounted, [16 x i8] }>*
40+
// CHECK: [[BINDINGSADDR:%.*]] = getelementptr inbounds <{ %swift.refcounted, [16 x i8] }>, <{ %swift.refcounted, [16 x i8] }>* [[CONTEXT]], i32 0, i32 1
4141
// CHECK: [[TYPEADDR:%.*]] = bitcast [16 x i8]* [[BINDINGSADDR]]
4242
// CHECK: [[TYPE:%.*]] = load %swift.type*, %swift.type** [[TYPEADDR]], align 8
4343
// CHECK: [[WITNESSADDR_0:%.*]] = getelementptr inbounds %swift.type*, %swift.type** [[TYPEADDR]], i32 1
4444
// CHECK: [[WITNESSADDR:%.*]] = bitcast %swift.type** [[WITNESSADDR_0]]
4545
// CHECK: [[WITNESS:%.*]] = load i8**, i8*** [[WITNESSADDR]], align 8
46-
// CHECK: [[BOXADDR:%.*]] = getelementptr inbounds <{ %swift.refcounted, [16 x i8], %swift.refcounted* }>, <{ %swift.refcounted, [16 x i8], %swift.refcounted* }>* [[CONTEXT]], i32 0, i32 2
47-
// CHECK: [[BOX:%.*]] = load %swift.refcounted*, %swift.refcounted** [[BOXADDR]], align 8
48-
// CHECK: [[RES:%.*]] = tail call swiftcc i64 @"$s7closure1b3seqS2icx_tAA9OrdinableRzlFS2icfU_"(i64 %0, %swift.refcounted* [[BOX]], %swift.type* [[TYPE]], i8** [[WITNESS]])
46+
// CHECK: [[RES:%.*]] = tail call swiftcc i64 @"$s7closure1b3seqS2icx_tAA9OrdinableRzlFS2icfU_"(i64 %0, %swift.opaque* noalias nocapture {{.*}}, %swift.type* [[TYPE]], i8** [[WITNESS]])
4947
// CHECK: ret i64 [[RES]]
5048
// CHECK: }
5149

@@ -55,9 +53,7 @@ func captures_tuple<T, U>(x x: (T, U)) -> () -> (T, U) {
5553
// CHECK: [[T0:%.*]] = call swiftcc %swift.metadata_response @swift_getTupleTypeMetadata2(i64 0, %swift.type* %T, %swift.type* %U, i8* null, i8** null)
5654
// CHECK-NEXT: [[METADATA:%.*]] = extractvalue %swift.metadata_response [[T0]], 0
5755
// CHECK-NOT: @swift_getTupleTypeMetadata2
58-
// CHECK: [[BOX:%.*]] = call swiftcc { %swift.refcounted*, %swift.opaque* } @swift_allocBox(%swift.type* [[METADATA]])
59-
// CHECK: [[ADDR:%.*]] = extractvalue { %swift.refcounted*, %swift.opaque* } [[BOX]], 1
60-
// CHECK: bitcast %swift.opaque* [[ADDR]] to <{}>*
56+
// CHECK: ret
6157
return {x}
6258
}
6359

‎test/SILGen/capture_resilience.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public func hasClosureVar() -> () -> ResilientStruct {
2929
@inlinable public func hasInlinableClosureLet() -> () -> ResilientStruct {
3030
let s = ResilientStruct()
3131

32-
// CHECK-LABEL: sil shared [serialized] [ossa] @$s18capture_resilience22hasInlinableClosureLetAA15ResilientStructVycyFADycfU_ : $@convention(thin) (@guaranteed { var ResilientStruct }) -> @out ResilientStruct
32+
// CHECK_LABEL: sil shared [serialized] [ossa] @$s18capture_resilience22hasInlinableClosureLetAA15ResilientStructVycyFADycfU_ : $@convention(thin) (@in_guaranteed ResilientStruct) -> @out ResilientStruct
3333
return { s }
3434
}
3535

@@ -63,7 +63,7 @@ public func hasNoEscapeClosureVar() {
6363
@inlinable public func hasInlinableNoEscapeClosureLet() {
6464
let s = ResilientStruct()
6565

66-
// CHECK-LABEL: sil shared [serialized] [ossa] @$s18capture_resilience30hasInlinableNoEscapeClosureLetyyFAA15ResilientStructVyXEfU_ : $@convention(thin) (@inout_aliasable ResilientStruct) -> @out ResilientStruct
66+
// CHECK-LABEL: sil shared [serialized] [ossa] @$s18capture_resilience30hasInlinableNoEscapeClosureLetyyFAA15ResilientStructVyXEfU_ : $@convention(thin) (@in_guaranteed ResilientStruct) -> @out ResilientStruct
6767
consume { s }
6868
}
6969

0 commit comments

Comments
 (0)