Skip to content

Commit 033fc23

Browse files
nate-chandlernate-chandler
authored
Merge pull request #74157 from nate-chandler/rdar128710064
[SILGen] Store_borrow into in_guaranteed.
2 parents 1c58d8b + c35b86a commit 033fc23

File tree

10 files changed

+187
-26
lines changed

10 files changed

+187
-26
lines changed

‎lib/SILGen/Initialization.h

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,13 +185,16 @@ class Initialization {
185185
/// of last resort: it is generally better to split tuples or evaluate
186186
/// in-place when the initialization supports that.
187187
///
188-
/// If this is an *copy* of the rvalue into this initialization then isInit is
188+
/// If this is a *copy* of the rvalue into this initialization then isInit is
189189
/// false. If it is an *initialization* of the memory in the initialization,
190190
/// then isInit is true.
191191
virtual void copyOrInitValueInto(SILGenFunction &SGF, SILLocation loc,
192192
ManagedValue explodedElement,
193193
bool isInit) = 0;
194194

195+
/// Whether the storage owns what's stored or merely borrows it.
196+
virtual bool isBorrow() { return false; }
197+
195198
/// Whether to emit a debug value during initialization.
196199
void setEmitDebugValueOnInit(bool emit) { EmitDebugValueOnInit = emit; }
197200

@@ -217,7 +220,7 @@ class Initialization {
217220

218221
/// Abstract base class for single-buffer initializations. These are
219222
/// initializations that have an addressable memory object to be stored into.
220-
class SingleBufferInitialization : public Initialization {
223+
class SingleBufferInitialization : virtual public Initialization {
221224
llvm::TinyPtrVector<CleanupHandle::AsPointer> SplitCleanups;
222225
public:
223226
SingleBufferInitialization() {}
@@ -262,7 +265,7 @@ class SingleBufferInitialization : public Initialization {
262265
SmallVectorImpl<InitializationPtr> &buf,
263266
TinyPtrVector<CleanupHandle::AsPointer> &splitCleanups);
264267
};
265-
268+
266269
/// This is an initialization for a specific address in memory.
267270
class KnownAddressInitialization : public SingleBufferInitialization {
268271
/// The physical address of the global.
@@ -285,7 +288,13 @@ class KnownAddressInitialization : public SingleBufferInitialization {
285288
void finishUninitialized(SILGenFunction &SGF) override {}
286289
};
287290

288-
class TemporaryInitialization : public SingleBufferInitialization {
291+
class AnyTemporaryInitialization : virtual public Initialization {
292+
public:
293+
virtual ManagedValue getManagedAddress() const = 0;
294+
};
295+
296+
class TemporaryInitialization : public SingleBufferInitialization,
297+
public AnyTemporaryInitialization {
289298
SILValue Addr;
290299
CleanupHandle Cleanup;
291300
public:
@@ -312,12 +321,38 @@ class TemporaryInitialization : public SingleBufferInitialization {
312321
/// Returns the cleanup corresponding to the value of the temporary.
313322
CleanupHandle getInitializedCleanup() const { return Cleanup; }
314323

315-
ManagedValue getManagedAddress() const {
324+
ManagedValue getManagedAddress() const override {
316325
return ManagedValue::forOwnedAddressRValue(getAddress(),
317326
getInitializedCleanup());
318327
}
319328
};
320329

330+
class StoreBorrowInitialization final : public AnyTemporaryInitialization {
331+
SILValue address;
332+
ManagedValue storeBorrow;
333+
334+
public:
335+
StoreBorrowInitialization(SILValue address);
336+
337+
void copyOrInitValueInto(SILGenFunction &SGF, SILLocation loc,
338+
ManagedValue mv, bool isInit) override;
339+
340+
void finishInitialization(SILGenFunction &SGF) override {}
341+
342+
void finishUninitialized(SILGenFunction &SGF) override {}
343+
344+
bool isBorrow() override { return true; }
345+
346+
SILValue getAddress() const;
347+
348+
bool isInPlaceInitializationOfGlobal() const override {
349+
// Can't store_borrow to a global.
350+
return false;
351+
}
352+
353+
ManagedValue getManagedAddress() const override;
354+
};
355+
321356
/// An initialization which accumulates several other initializations
322357
/// into a tuple.
323358
class TupleInitialization : public Initialization {

‎lib/SILGen/ManagedValue.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ void ManagedValue::assignInto(SILGenFunction &SGF, SILLocation loc,
170170

171171
void ManagedValue::forwardInto(SILGenFunction &SGF, SILLocation loc,
172172
Initialization *dest) {
173-
assert(isPlusOneOrTrivial(SGF));
173+
assert(isPlusOneOrTrivial(SGF) || dest->isBorrow());
174174
dest->copyOrInitValueInto(SGF, loc, *this, /*isInit*/ true);
175175
dest->finishInitialization(SGF);
176176
}

‎lib/SILGen/SILGenDecl.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,43 @@ void TemporaryInitialization::finishInitialization(SILGenFunction &SGF) {
367367
SGF.Cleanups.setCleanupState(Cleanup, CleanupState::Active);
368368
}
369369

370+
StoreBorrowInitialization::StoreBorrowInitialization(SILValue address)
371+
: address(address) {
372+
assert(isa<AllocStackInst>(address) ||
373+
isa<MarkUnresolvedNonCopyableValueInst>(address) &&
374+
"invalid destination for store_borrow initialization!?");
375+
}
376+
377+
void StoreBorrowInitialization::copyOrInitValueInto(SILGenFunction &SGF,
378+
SILLocation loc,
379+
ManagedValue mv,
380+
bool isInit) {
381+
auto value = mv.getValue();
382+
auto &lowering = SGF.getTypeLowering(value->getType());
383+
if (lowering.isAddressOnly() && SGF.silConv.useLoweredAddresses()) {
384+
llvm::report_fatal_error(
385+
"Attempting to store_borrow an address-only value!?");
386+
}
387+
if (value->getType().isAddress()) {
388+
value = SGF.emitManagedLoadBorrow(loc, value).getValue();
389+
}
390+
if (!isInit) {
391+
value = lowering.emitCopyValue(SGF.B, loc, value);
392+
}
393+
storeBorrow = SGF.emitManagedStoreBorrow(loc, value, address);
394+
}
395+
396+
SILValue StoreBorrowInitialization::getAddress() const {
397+
if (storeBorrow) {
398+
return storeBorrow.getValue();
399+
}
400+
return address;
401+
}
402+
403+
ManagedValue StoreBorrowInitialization::getManagedAddress() const {
404+
return storeBorrow;
405+
}
406+
370407
namespace {
371408
class ReleaseValueCleanup : public Cleanup {
372409
SILValue v;

‎lib/SILGen/SILGenPoly.cpp

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,6 +1080,33 @@ class ParamInfo {
10801080
IndirectSlot slot;
10811081
ParameterConvention convention;
10821082

1083+
bool temporaryShouldBorrow(SILGenFunction &SGF, bool forceAllocation) {
1084+
if (slot.hasAddress() && !forceAllocation) {
1085+
// An address has already been allocated via some projection. It is not
1086+
// currently supported to store_borrow to such projections.
1087+
return false;
1088+
}
1089+
auto &tl = getTypeLowering(SGF);
1090+
if (tl.isAddressOnly() && SGF.silConv.useLoweredAddresses()) {
1091+
// In address-lowered mode, address-only types can't be loaded in the
1092+
// first place before being store_borrow'd.
1093+
return false;
1094+
}
1095+
if (convention != ParameterConvention::Indirect_In_Guaranteed) {
1096+
// Can only store_borrow into a temporary allocation for @in_guaranteed.
1097+
return false;
1098+
}
1099+
if (tl.isTrivial()) {
1100+
// Can't store_borrow a trivial type.
1101+
return false;
1102+
}
1103+
return true;
1104+
}
1105+
1106+
TypeLowering const &getTypeLowering(SILGenFunction &SGF) {
1107+
return SGF.getTypeLowering(getType());
1108+
}
1109+
10831110
public:
10841111
ParamInfo(IndirectSlot slot, ParameterConvention convention)
10851112
: slot(slot), convention(convention) {}
@@ -1088,11 +1115,26 @@ class ParamInfo {
10881115
return slot.allocate(SGF, loc);
10891116
}
10901117

1091-
std::unique_ptr<TemporaryInitialization>
1092-
allocateForInitialization(SILGenFunction &SGF, SILLocation loc) const {
1093-
auto addr = slot.allocate(SGF, loc);
1094-
auto &addrTL = SGF.getTypeLowering(addr->getType());
1095-
return SGF.useBufferAsTemporary(addr, addrTL);
1118+
std::unique_ptr<AnyTemporaryInitialization>
1119+
allocateForInitialization(SILGenFunction &SGF, SILLocation loc,
1120+
bool forceAllocation = false) {
1121+
auto &lowering = getTypeLowering(SGF);
1122+
SILValue address;
1123+
if (forceAllocation) {
1124+
address = SGF.emitTemporaryAllocation(loc, lowering.getLoweredType());
1125+
} else {
1126+
address = allocate(SGF, loc);
1127+
}
1128+
if (address->getType().isMoveOnly())
1129+
address = SGF.B.createMarkUnresolvedNonCopyableValueInst(
1130+
loc, address,
1131+
MarkUnresolvedNonCopyableValueInst::CheckKind::
1132+
ConsumableAndAssignable);
1133+
if (temporaryShouldBorrow(SGF, forceAllocation)) {
1134+
return std::make_unique<StoreBorrowInitialization>(address);
1135+
}
1136+
auto innerTemp = SGF.useBufferAsTemporary(address, lowering);
1137+
return innerTemp;
10961138
}
10971139

10981140
SILType getType() const {
@@ -1904,8 +1946,8 @@ class TranslateArguments : public ExpanderBase<TranslateArguments, ParamInfo> {
19041946
ManagedValue outerArg,
19051947
ParamInfo innerSlot) {
19061948
auto innerResultTy = innerSlot.getType();
1907-
auto &innerTL = SGF.getTypeLowering(innerResultTy);
1908-
auto innerTemp = SGF.emitTemporary(Loc, innerTL);
1949+
auto innerTemp =
1950+
innerSlot.allocateForInitialization(SGF, Loc, /*forceAllocation=*/true);
19091951
processSingleInto(innerOrigType, innerSubstType,
19101952
outerOrigType, outerSubstType,
19111953
outerArg, innerResultTy.getAddressType(),
@@ -2139,7 +2181,7 @@ class TranslateArguments : public ExpanderBase<TranslateArguments, ParamInfo> {
21392181
outer, innerTy,
21402182
SGFContext(&init));
21412183
if (!innerArg.isInContext()) {
2142-
if (innerArg.isPlusOneOrTrivial(SGF)) {
2184+
if (innerArg.isPlusOneOrTrivial(SGF) || init.isBorrow()) {
21432185
innerArg.forwardInto(SGF, Loc, &init);
21442186
} else {
21452187
innerArg.copyInto(SGF, Loc, &init);

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3044,6 +3044,13 @@ static void insertDestroyBeforeInstruction(UseState &addressUseState,
30443044
SILValue baseAddress,
30453045
SmallBitVector &bv,
30463046
ConsumeInfo &consumes) {
3047+
if (!baseAddress->getUsersOfType<StoreBorrowInst>().empty()) {
3048+
// If there are _any_ store_borrow users, then all users of the address are
3049+
// store_borrows (and dealloc_stacks). Nothing is stored, there's nothing
3050+
// to destroy.
3051+
return;
3052+
}
3053+
30473054
// If we need all bits...
30483055
if (bv.all()) {
30493056
// And our next instruction is a destroy_addr on the base address, just

‎test/SILGen/protocol_resilience.swift

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -220,15 +220,14 @@ extension ReabstractSelfBase {
220220

221221
// CHECK-LABEL: sil private [transparent] [thunk] [ossa] @$s19protocol_resilience21ReabstractSelfRefinedP8callbackyxxcvg :
222222
// CHECK: [[SELF_BOX:%.*]] = alloc_stack $τ_0_0
223-
// CHECK-NEXT: [[SELF_COPY:%.*]] = copy_value %0 : $τ_0_0
224-
// CHECK-NEXT: store [[SELF_COPY]] to [init] [[SELF_BOX]] : $*τ_0_0
223+
// CHECK-NEXT: [[SELF_BOX_BORROW:%.*]] = store_borrow %0 to [[SELF_BOX]]
225224
// CHECK: [[WITNESS:%.*]] = function_ref @$s19protocol_resilience18ReabstractSelfBasePAAE8callbackyxxcvg
226-
// CHECK-NEXT: [[RESULT:%.*]] = apply [[WITNESS]]<τ_0_0>([[SELF_BOX]])
225+
// CHECK-NEXT: [[RESULT:%.*]] = apply [[WITNESS]]<τ_0_0>([[SELF_BOX_BORROW]])
227226
// CHECK-NEXT: [[RESULT_CONV:%.*]] = convert_function [[RESULT]]
228227
// CHECK: [[THUNK_FN:%.*]] = function_ref
229228
// CHECK-NEXT: [[THUNK:%.*]] = partial_apply [callee_guaranteed] [[THUNK_FN]]<τ_0_0>([[RESULT_CONV]])
230229
// CHECK-NEXT: [[THUNK_CONV:%.*]] = convert_function [[THUNK]]
231-
// CHECK-NEXT: destroy_addr [[SELF_BOX]]
230+
// CHECK-NEXT: end_borrow [[SELF_BOX_BORROW]]
232231
// CHECK-NEXT: dealloc_stack [[SELF_BOX]]
233232
// CHECK-NEXT: return [[THUNK_CONV]]
234233

‎test/SILGen/variadic-generic-reabstract-tuple-arg.swift

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,14 @@ func test1() {
3636
// CHECK-NEXT: [[PACK:%.*]] = alloc_pack $Pack{String}
3737
// CHECK-NEXT: [[ARG_TEMP:%.*]] = alloc_stack $String
3838
// It'd be nice to avoid this unnecessary copy.
39-
// CHECK-NEXT: [[ARG_COPY:%.*]] = copy_value %0 : $String
40-
// CHECK-NEXT: store [[ARG_COPY]] to [init] [[ARG_TEMP]] : $*String
39+
// CHECK-NEXT: [[ARG_TEMP_BORROW:%.*]] = store_borrow %0 to [[ARG_TEMP]]
4140
// CHECK-NEXT: [[INDEX:%.*]] = scalar_pack_index 0 of $Pack{String}
42-
// CHECK-NEXT: pack_element_set [[ARG_TEMP]] : $*String into [[INDEX]] of [[PACK]] : $*Pack{String}
41+
// CHECK-NEXT: pack_element_set [[ARG_TEMP_BORROW]] : $*String into [[INDEX]] of [[PACK]] : $*Pack{String}
4342
// CHECK-NEXT: [[RESULT_TEMP:%.*]] = alloc_stack $Array<String>
4443
// CHECK-NEXT: apply %1([[RESULT_TEMP]], [[PACK]]) : $@callee_guaranteed (@pack_guaranteed Pack{String}) -> @out Array<String>
4544
// CHECK-NEXT: [[RESULT:%.*]] = load [take] [[RESULT_TEMP]] : $*Array<String>
4645
// CHECK-NEXT: dealloc_stack [[RESULT_TEMP]] : $*Array<String>
47-
// CHECK-NEXT: destroy_addr [[ARG_TEMP]] : $*String
46+
// CHECK-NEXT: end_borrow [[ARG_TEMP_BORROW]]
4847
// CHECK-NEXT: dealloc_stack [[ARG_TEMP]] : $*String
4948
// CHECK-NEXT: dealloc_pack [[PACK]] : $*Pack{String}
5049
// CHECK-NEXT: return [[RESULT]] : $Array<String>

‎test/SILGen/witnesses.swift

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -250,15 +250,14 @@ struct ConformsWithMoreGeneric : X, Y {
250250
// CHECK-LABEL: sil private [transparent] [thunk] [ossa] @$s9witnesses23ConformsWithMoreGenericVAA1XA2aDP7classes{{[_0-9a-zA-Z]*}}FTW :
251251
// CHECK: bb0([[ARG0:%.*]] : @guaranteed $τ_0_0, [[ARG1:%.*]] : $*ConformsWithMoreGeneric):
252252
// CHECK-NEXT: [[SELF_BOX:%.*]] = alloc_stack $τ_0_0
253-
// CHECK-NEXT: [[ARG0_COPY:%.*]] = copy_value [[ARG0]]
254-
// CHECK-NEXT: store [[ARG0_COPY]] to [init] [[SELF_BOX]] : $*τ_0_0
253+
// CHECK-NEXT: [[SELF_BOX_BORROW:%.*]] = store_borrow [[ARG0]] to [[SELF_BOX]]
255254
// CHECK-NEXT: // function_ref witnesses.ConformsWithMoreGeneric.classes
256255
// CHECK-NEXT: [[WITNESS_FN:%.*]] = function_ref @$s9witnesses23ConformsWithMoreGenericV7classes{{[_0-9a-zA-Z]*}}F : $@convention(method) <τ_0_0> (@in_guaranteed τ_0_0, @inout ConformsWithMoreGeneric) -> @out τ_0_0
257256
// CHECK-NEXT: [[RESULT_BOX:%.*]] = alloc_stack $τ_0_0
258-
// CHECK-NEXT: [[RESULT:%.*]] = apply [[WITNESS_FN]]<τ_0_0>([[RESULT_BOX]], [[SELF_BOX]], %1) : $@convention(method) <τ_0_0> (@in_guaranteed τ_0_0, @inout ConformsWithMoreGeneric) -> @out τ_0_0
257+
// CHECK-NEXT: [[RESULT:%.*]] = apply [[WITNESS_FN]]<τ_0_0>([[RESULT_BOX]], [[SELF_BOX_BORROW]], %1) : $@convention(method) <τ_0_0> (@in_guaranteed τ_0_0, @inout ConformsWithMoreGeneric) -> @out τ_0_0
259258
// CHECK-NEXT: [[RESULT:%.*]] = load [take] [[RESULT_BOX]] : $*τ_0_0
260259
// CHECK-NEXT: dealloc_stack [[RESULT_BOX]] : $*τ_0_0
261-
// CHECK-NEXT: destroy_addr [[SELF_BOX]]
260+
// CHECK-NEXT: end_borrow [[SELF_BOX_BORROW]]
262261
// CHECK-NEXT: dealloc_stack [[SELF_BOX]] : $*τ_0_0
263262
// CHECK-NEXT: return [[RESULT]] : $τ_0_0
264263
// CHECK-NEXT: }

‎test/SILOptimizer/moveonly_addresschecker_unmaximized.sil

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ struct M4: ~Copyable {
2323

2424
sil @get_M4 : $@convention(thin) () -> @owned M4
2525
sil @end_2 : $@convention(thin) (@owned M, @owned M) -> ()
26+
sil @see_addr : $@convention(thin) (@in_guaranteed M) -> ()
2627
sil @see_addr_2 : $@convention(thin) (@in_guaranteed M, @in_guaranteed M) -> ()
2728
sil @replace_2 : $@convention(thin) (@inout M, @inout M) -> ()
2829
sil @get_out_2 : $@convention(thin) () -> (@out M, @out M)
@@ -235,3 +236,20 @@ bb0(%m_in : @owned $M):
235236
apply %die(%pa) : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> @owned String) -> Never
236237
unreachable
237238
}
239+
240+
// CHECK-LABEL: sil [ossa] @dont_destroy_store_borrowed_addr : {{.*}} {
241+
// CHECK-NOT: destroy_addr
242+
// CHECK-LABEL: } // end sil function 'dont_destroy_store_borrowed_addr'
243+
sil [ossa] @dont_destroy_store_borrowed_addr : $@convention(thin) (@guaranteed M) -> () {
244+
bb0(%0 : @guaranteed $M):
245+
%stack = alloc_stack $M
246+
%mark = mark_unresolved_non_copyable_value [consumable_and_assignable] %stack : $*M
247+
%borrow = store_borrow %0 to %mark : $*M
248+
%see_addr = function_ref @see_addr : $@convention(thin) (@in_guaranteed M) -> ()
249+
apply %see_addr(%borrow) : $@convention(thin) (@in_guaranteed M) -> ()
250+
end_borrow %borrow : $*M
251+
dealloc_stack %stack : $*M
252+
%retval = tuple ()
253+
return %retval : $()
254+
}
255+

‎validation-test/SILGen/rdar128710064.swift

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// RUN: %target-swift-emit-silgen %s | %FileCheck %s
2+
// RUN: %target-swift-emit-sil %s -verify
3+
4+
struct NC : ~Copyable {}
5+
6+
func loadClosure(_ umrp: UnsafeMutableRawPointer) {
7+
typealias Enumerator = (borrowing NC) -> Void
8+
let body = umrp.load(as: Enumerator.self)
9+
_ = body
10+
}
11+
12+
// CHECK-LABEL: sil {{.*}} @$s13rdar1287100642NCVytIegnr_ACIegg_TR : {{.*}} {
13+
// CHECK: bb0(
14+
// CHECK-SAME: [[NC:%[^,]+]] :
15+
// CHECK-SAME: , [[CLOSURE:%[^,]+]] :
16+
// CHECK-SAME: ):
17+
// CHECK: [[NC_INDIRECT_ADDR:%[^,]+]] = alloc_stack $NC
18+
// CHECK: [[NC_INDIRECT_CHECK:%[^,]+]] = mark_unresolved_non_copyable_value [consumable_and_assignable] [[NC_INDIRECT_ADDR]]
19+
// CHECK: [[NC_INDIRECT:%[^,]+]] = store_borrow [[NC]] to [[NC_INDIRECT_CHECK]]
20+
// CHECK: [[OUT:%[^,]+]] = alloc_stack $()
21+
// CHECK: apply [[CLOSURE]]([[OUT]], [[NC_INDIRECT]])
22+
// CHECK: dealloc_stack [[OUT]] : $*()
23+
// CHECK: end_borrow [[NC_INDIRECT]]
24+
// CHECK: dealloc_stack [[NC_INDIRECT_ADDR]]
25+
// CHECK-LABEL: } // end sil function '$s13rdar1287100642NCVytIegnr_ACIegg_TR'

0 commit comments

Comments
 (0)