Skip to content

Commit 5a15f88

Browse files
gottesmmgottesmm
committed
[silgen] Change a bunch of self accesses to use true formal evaluation scopes and formal accesses.
This is in preparation for removing the +0 self hack. This commit in more detail does the following: 1. It adds Formal Evaluation Scopes to certain places where the scopes were missing. Specifically: a. The SILGenLValue cases are places where we are invoking accessors. In each one of these cases, we had a formal evaluation scope in the accessor itself, but we did not have a scope that closed over the base access and the accessor access. The base access is a formal evaluation in the sense that just like with inout bases, we must create a new reference to the base and re-destroy the base in a chain of accesses. This is to ensure that we do not extend the lifetime of the base inappropriately. b. The SILGenPoly case is a place where we have never properly placed a Formal Evaluation Scope and have completely been relying on the +0 self hack to make sure that archetype callees are properly destroyed immediately after a +0 call. 2. It changes all of the places in SILGen that emit self to using formal access cleanups instead of normal cleanups. rdar://29791263
1 parent dfd3b74 commit 5a15f88

12 files changed

+150
-23
lines changed

‎lib/SILGen/ManagedValue.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,19 @@ ManagedValue ManagedValue::copyUnmanaged(SILGenFunction &gen, SILLocation loc) {
8181
return gen.emitManagedRValueWithCleanup(result);
8282
}
8383

84+
/// This is the same operation as 'copy', but works on +0 values that don't
85+
/// have cleanups. It returns a +1 value with one.
86+
ManagedValue ManagedValue::formalAccessCopyUnmanaged(SILGenFunction &gen,
87+
SILLocation loc) {
88+
if (getType().isObject()) {
89+
return gen.B.createFormalAccessCopyValue(loc, *this);
90+
}
91+
92+
SILValue result = gen.emitTemporaryAllocation(loc, getType());
93+
return gen.B.createFormalAccessCopyAddr(loc, *this, result, IsNotTake,
94+
IsInitialization);
95+
}
96+
8497
/// Disable the cleanup for this value.
8598
void ManagedValue::forwardCleanup(SILGenFunction &gen) const {
8699
assert(hasCleanup() && "value doesn't have cleanup!");

‎lib/SILGen/ManagedValue.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,11 @@ class ManagedValue {
250250
/// This is the same operation as 'copy', but works on +0 values that don't
251251
/// have cleanups. It returns a +1 value with one.
252252
ManagedValue copyUnmanaged(SILGenFunction &gen, SILLocation loc);
253-
253+
254+
/// This is the same operation as 'formalAccessCopy', but works on +0 values
255+
/// that don't have cleanups. It returns a +1 value with one.
256+
ManagedValue formalAccessCopyUnmanaged(SILGenFunction &gen, SILLocation loc);
257+
254258
bool hasCleanup() const { return cleanup.isValid(); }
255259
CleanupHandle getCleanup() const { return cleanup; }
256260

‎lib/SILGen/SILGenApply.cpp

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -639,10 +639,11 @@ class Callee {
639639
/// the original had a cleanup.
640640
static ManagedValue maybeEnterCleanupForTransformed(SILGenFunction &gen,
641641
ManagedValue orig,
642-
SILValue result) {
642+
SILValue result,
643+
SILLocation loc) {
643644
if (orig.hasCleanup()) {
644645
orig.forwardCleanup(gen);
645-
return gen.emitManagedBufferWithCleanup(result);
646+
return gen.emitFormalAccessManagedBufferWithCleanup(loc, result);
646647
} else {
647648
return ManagedValue::forUnmanaged(result);
648649
}
@@ -763,7 +764,7 @@ class ArchetypeCalleeBuilder {
763764
StoreOwnershipQualifier::Init);
764765

765766
// If we had a cleanup, create a cleanup at the new address.
766-
return maybeEnterCleanupForTransformed(gen, ref, temp);
767+
return maybeEnterCleanupForTransformed(gen, ref, temp, selfLoc);
767768
}
768769
};
769770

@@ -2750,7 +2751,8 @@ namespace {
27502751
assert(destAddr->getType() == loweredSubstParamType.getAddressType());
27512752

27522753
auto &destTL = SharedInfo->getBaseTypeLowering();
2753-
Cleanup = gen.enterDormantTemporaryCleanup(destAddr, destTL);
2754+
Cleanup =
2755+
gen.enterDormantFormalAccessTemporaryCleanup(destAddr, loc, destTL);
27542756

27552757
TemporaryInitialization init(destAddr, Cleanup);
27562758
std::move(arg).forwardInto(gen, SharedInfo->getBaseAbstractionPattern(),
@@ -4337,6 +4339,7 @@ namespace {
43374339
AbstractionPattern origFormalType(callee.getOrigFormalType());
43384340
CanFunctionType formalType = callee.getSubstFormalType();
43394341

4342+
// Get the callee type information.
43404343
if (specializedEmitter || isPartiallyAppliedSuperMethod(uncurryLevel)) {
43414344
// We want to emit the arguments as fully-substituted values
43424345
// because that's what the specialized emitters expect.
@@ -5134,6 +5137,11 @@ emitSpecializedAccessorFunctionRef(SILGenFunction &gen,
51345137

51355138
namespace {
51365139

5140+
/// A builder class that creates the base argument for accessors.
5141+
///
5142+
/// *NOTE* All cleanups created inside of this builder on base arguments must be
5143+
/// formal access to ensure that we do not extend the lifetime of a guaranteed
5144+
/// base after the accessor is evaluated.
51375145
struct AccessorBaseArgPreparer final {
51385146
SILGenFunction &SGF;
51395147
SILLocation loc;
@@ -5200,9 +5208,9 @@ ArgumentSource AccessorBaseArgPreparer::prepareAccessorAddressBaseArg() {
52005208
// The load can only be a take if the base is a +1 rvalue.
52015209
auto shouldTake = IsTake_t(base.hasCleanup());
52025210

5203-
base = SGF.emitLoad(loc, base.forward(SGF),
5204-
SGF.getTypeLowering(baseLoweredType), SGFContext(),
5205-
shouldTake);
5211+
base = SGF.emitFormalAccessLoad(loc, base.forward(SGF),
5212+
SGF.getTypeLowering(baseLoweredType),
5213+
SGFContext(), shouldTake);
52065214
return ArgumentSource(loc, RValue(SGF, loc, baseFormalType, base));
52075215
}
52085216

@@ -5239,7 +5247,7 @@ ArgumentSource AccessorBaseArgPreparer::prepareAccessorObjectBaseArg() {
52395247

52405248
// We need to produce the value at +1 if it's going to be consumed.
52415249
if (selfParam.isConsumed() && !base.hasCleanup()) {
5242-
base = base.copyUnmanaged(SGF, loc);
5250+
base = base.formalAccessCopyUnmanaged(SGF, loc);
52435251
}
52445252

52455253
// If the parameter is indirect, we need to drop the value into

‎lib/SILGen/SILGenBuilder.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,36 @@ ManagedValue SILGenBuilder::bufferForExpr(
354354
return gen.emitManagedBufferWithCleanup(address);
355355
}
356356

357+
358+
ManagedValue SILGenBuilder::formalAccessBufferForExpr(
359+
SILLocation loc, SILType ty, const TypeLowering &lowering,
360+
SGFContext context, std::function<void(SILValue)> rvalueEmitter) {
361+
// If we have a single-buffer "emit into" initialization, use that for the
362+
// result.
363+
SILValue address = context.getAddressForInPlaceInitialization();
364+
365+
// If we couldn't emit into the Initialization, emit into a temporary
366+
// allocation.
367+
if (!address) {
368+
address = gen.emitTemporaryAllocation(loc, ty.getObjectType());
369+
}
370+
371+
rvalueEmitter(address);
372+
373+
// If we have a single-buffer "emit into" initialization, use that for the
374+
// result.
375+
if (context.getAddressForInPlaceInitialization()) {
376+
context.getEmitInto()->finishInitialization(gen);
377+
return ManagedValue::forInContext();
378+
}
379+
380+
// Add a cleanup for the temporary we allocated.
381+
if (lowering.isTrivial())
382+
return ManagedValue::forUnmanaged(address);
383+
384+
return gen.emitFormalAccessManagedBufferWithCleanup(loc, address);
385+
}
386+
357387
ManagedValue SILGenBuilder::createUncheckedEnumData(SILLocation loc,
358388
ManagedValue operand,
359389
EnumElementDecl *element) {
@@ -458,3 +488,4 @@ ManagedValue SILGenBuilder::createEnum(SILLocation loc, ManagedValue payload,
458488
return ManagedValue::forUnmanaged(result);
459489
return gen.emitManagedRValueWithCleanup(result);
460490
}
491+

‎lib/SILGen/SILGenBuilder.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,10 @@ class SILGenBuilder : public SILBuilder {
207207
EnumElementDecl *decl, SILType type);
208208

209209
ManagedValue createSemanticLoadBorrow(SILLocation loc, ManagedValue addr);
210+
211+
ManagedValue formalAccessBufferForExpr(
212+
SILLocation loc, SILType ty, const TypeLowering &lowering,
213+
SGFContext context, std::function<void(SILValue)> rvalueEmitter);
210214
};
211215

212216
} // namespace Lowering

‎lib/SILGen/SILGenFunction.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1215,7 +1215,12 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
12151215
const TypeLowering &rvalueTL,
12161216
SGFContext C, IsTake_t isTake,
12171217
bool isGuaranteedValid = false);
1218-
1218+
1219+
ManagedValue emitFormalAccessLoad(SILLocation loc, SILValue addr,
1220+
const TypeLowering &rvalueTL, SGFContext C,
1221+
IsTake_t isTake,
1222+
bool isGuaranteedValid = false);
1223+
12191224
void emitAssignToLValue(SILLocation loc, RValue &&src,
12201225
LValue &&dest);
12211226
void emitAssignLValueToLValue(SILLocation loc,

‎lib/SILGen/SILGenLValue.cpp

Lines changed: 69 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -694,10 +694,11 @@ namespace {
694694
RValue &&value, ManagedValue base) && override {
695695
SILDeclRef setter = gen.getSetterDeclRef(decl, IsDirectAccessorUse);
696696

697+
FormalEvaluationScope scope(gen);
697698
// Pass in just the setter.
698699
auto args =
699700
std::move(*this).prepareAccessorArgs(gen, loc, base, setter);
700-
701+
701702
return gen.emitSetAccessor(loc, setter, substitutions,
702703
std::move(args.base), IsSuper,
703704
IsDirectAccessorUse,
@@ -940,6 +941,8 @@ namespace {
940941
ManagedValue base, SGFContext c) && override {
941942
SILDeclRef getter = gen.getGetterDeclRef(decl, IsDirectAccessorUse);
942943

944+
FormalEvaluationScope scope(gen);
945+
943946
auto args =
944947
std::move(*this).prepareAccessorArgs(gen, loc, base, getter);
945948

@@ -1131,13 +1134,16 @@ namespace {
11311134

11321135
SILDeclRef addressor = gen.getAddressorDeclRef(decl, accessKind,
11331136
IsDirectAccessorUse);
1134-
auto args =
1135-
std::move(*this).prepareAccessorArgs(gen, loc, base, addressor);
1136-
auto result = gen.emitAddressorAccessor(loc, addressor, substitutions,
1137-
std::move(args.base), IsSuper,
1138-
IsDirectAccessorUse,
1139-
std::move(args.subscripts),
1140-
SubstFieldType);
1137+
std::pair<ManagedValue, ManagedValue> result;
1138+
{
1139+
FormalEvaluationScope scope(gen);
1140+
1141+
auto args =
1142+
std::move(*this).prepareAccessorArgs(gen, loc, base, addressor);
1143+
result = gen.emitAddressorAccessor(
1144+
loc, addressor, substitutions, std::move(args.base), IsSuper,
1145+
IsDirectAccessorUse, std::move(args.subscripts), SubstFieldType);
1146+
}
11411147
switch (cast<FuncDecl>(addressor.getDecl())->getAddressorKind()) {
11421148
case AddressorKind::NotAddressor:
11431149
llvm_unreachable("not an addressor!");
@@ -2053,12 +2059,65 @@ ManagedValue SILGenFunction::emitLoad(SILLocation loc, SILValue addr,
20532059
// we can perform a +0 load of the address instead of materializing a +1
20542060
// value.
20552061
if (isPlusZeroOk && addrTL.getLoweredType() == rvalueTL.getLoweredType()) {
2056-
return ManagedValue::forUnmanaged(B.createLoadBorrow(loc, addr));
2062+
return B.createLoadBorrow(loc, ManagedValue::forUnmanaged(addr));
2063+
}
2064+
2065+
// Load the loadable value, and retain it if we aren't taking it.
2066+
SILValue loadedV = emitSemanticLoad(loc, addr, addrTL, rvalueTL, isTake);
2067+
return emitManagedRValueWithCleanup(loadedV);
2068+
}
2069+
2070+
/// Load an r-value out of the given address.
2071+
///
2072+
/// \param rvalueTL - the type lowering for the type-of-rvalue
2073+
/// of the address
2074+
/// \param isGuaranteedValid - true if the value in this address
2075+
/// is guaranteed to be valid for the duration of the current
2076+
/// evaluation (see SGFContext::AllowGuaranteedPlusZero)
2077+
ManagedValue SILGenFunction::emitFormalAccessLoad(SILLocation loc,
2078+
SILValue addr,
2079+
const TypeLowering &rvalueTL,
2080+
SGFContext C, IsTake_t isTake,
2081+
bool isGuaranteedValid) {
2082+
// Get the lowering for the address type. We can avoid a re-lookup
2083+
// in the very common case of this being equivalent to the r-value
2084+
// type.
2085+
auto &addrTL = (addr->getType() == rvalueTL.getLoweredType().getAddressType()
2086+
? rvalueTL
2087+
: getTypeLowering(addr->getType()));
2088+
2089+
// Never do a +0 load together with a take.
2090+
bool isPlusZeroOk =
2091+
(isTake == IsNotTake && (isGuaranteedValid ? C.isGuaranteedPlusZeroOk()
2092+
: C.isImmediatePlusZeroOk()));
2093+
2094+
if (rvalueTL.isAddressOnly()) {
2095+
// If the client is cool with a +0 rvalue, the decl has an address-only
2096+
// type, and there are no conversions, then we can return this as a +0
2097+
// address RValue.
2098+
if (isPlusZeroOk && rvalueTL.getLoweredType() == addrTL.getLoweredType())
2099+
return ManagedValue::forUnmanaged(addr);
2100+
2101+
// Copy the address-only value.
2102+
return B.formalAccessBufferForExpr(
2103+
loc, rvalueTL.getLoweredType(), rvalueTL, C,
2104+
[&](SILValue addressForCopy) {
2105+
emitSemanticLoadInto(loc, addr, addrTL, addressForCopy, rvalueTL,
2106+
isTake, IsInitialization);
2107+
});
2108+
}
2109+
2110+
// Ok, this is something loadable. If this is a non-take access at plus zero,
2111+
// we can perform a +0 load of the address instead of materializing a +1
2112+
// value.
2113+
if (isPlusZeroOk && addrTL.getLoweredType() == rvalueTL.getLoweredType()) {
2114+
return B.createFormalAccessLoadBorrow(loc,
2115+
ManagedValue::forUnmanaged(addr));
20572116
}
20582117

20592118
// Load the loadable value, and retain it if we aren't taking it.
20602119
SILValue loadedV = emitSemanticLoad(loc, addr, addrTL, rvalueTL, isTake);
2061-
return emitManagedRValueWithCleanup(loadedV, rvalueTL);
2120+
return emitFormalAccessManagedRValueWithCleanup(loc, loadedV);
20622121
}
20632122

20642123
static void emitUnloweredStoreOfCopy(SILGenBuilder &B, SILLocation loc,

‎lib/SILGen/SILGenPoly.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3121,7 +3121,8 @@ void SILGenFunction::emitProtocolWitness(Type selfType,
31213121

31223122
SILLocation loc(witness.getDecl());
31233123
FullExpr scope(Cleanups, CleanupLocation::get(loc));
3124-
3124+
FormalEvaluationScope formalEvalScope(*this);
3125+
31253126
auto witnessKind = getWitnessDispatchKind(selfType, witness, isFree);
31263127
auto thunkTy = F.getLoweredFunctionType();
31273128

‎test/SILGen/errors.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,7 @@ class BaseThrowingInit : HasThrowingInit {
505505
// CHECK: [[T0:%.*]] = load_borrow [[MARKED_BOX]]
506506
// CHECK-NEXT: [[T1:%.*]] = ref_element_addr [[T0]] : $BaseThrowingInit, #BaseThrowingInit.subField
507507
// CHECK-NEXT: assign %1 to [[T1]]
508+
// CHECK-NEXT: end_borrow [[T0]] from [[MARKED_BOX]]
508509
// Super delegation.
509510
// CHECK-NEXT: [[T0:%.*]] = load [take] [[MARKED_BOX]]
510511
// CHECK-NEXT: [[T2:%.*]] = upcast [[T0]] : $BaseThrowingInit to $HasThrowingInit

‎test/SILGen/guaranteed_self.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,7 @@ class LetFieldClass {
481481
// CHECK-NEXT: [[KRAKEN:%.*]] = load_borrow [[KRAKEN_ADDR]]
482482
// CHECK-NEXT: [[KRAKEN_METH:%.*]] = class_method [[KRAKEN]]
483483
// CHECK-NEXT: apply [[KRAKEN_METH]]([[KRAKEN]])
484+
// CHECK-NEXT: end_borrow [[KRAKEN]] from [[KRAKEN_ADDR]]
484485
// CHECK-NEXT: [[KRAKEN_ADDR:%.*]] = ref_element_addr [[CLS]] : $LetFieldClass, #LetFieldClass.letk
485486
// CHECK-NEXT: [[KRAKEN:%.*]] = load [copy] [[KRAKEN_ADDR]]
486487
// CHECK: [[DESTROY_SHIP_FUN:%.*]] = function_ref @_T015guaranteed_self11destroyShipyAA6KrakenCF : $@convention(thin) (@owned Kraken) -> ()

‎test/SILGen/let_decls.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ struct LetPropertyStruct {
470470
// CHECK: [[ABOX:%[0-9]+]] = alloc_box ${ var LetPropertyStruct }
471471
// CHECK: [[A:%[0-9]+]] = project_box [[ABOX]]
472472
// CHECK: store %0 to [trivial] [[A]] : $*LetPropertyStruct
473-
// CHECK: [[STRUCT:%[0-9]+]] = load_borrow [[A]] : $*LetPropertyStruct
473+
// CHECK: [[STRUCT:%[0-9]+]] = load [trivial] [[A]] : $*LetPropertyStruct
474474
// CHECK: [[PROP:%[0-9]+]] = struct_extract [[STRUCT]] : $LetPropertyStruct, #LetPropertyStruct.lp
475475
// CHECK: destroy_value [[ABOX]] : ${ var LetPropertyStruct }
476476
// CHECK: return [[PROP]] : $Int

‎test/SILGen/struct_resilience.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ public func functionWithMyResilientTypes(_ s: MySize, f: (MySize) -> MySize) ->
144144
s2.w = s.w
145145

146146
// CHECK: [[RESULT_ADDR:%.*]] = struct_element_addr %1 : $*MySize, #MySize.h
147-
// CHECK: [[RESULT:%.*]] = load_borrow [[RESULT_ADDR]] : $*Int
147+
// CHECK: [[RESULT:%.*]] = load [trivial] [[RESULT_ADDR]] : $*Int
148148
_ = s.h
149149

150150
// CHECK: [[BORROWED_CLOSURE:%.*]] = begin_borrow %2

0 commit comments

Comments
 (0)