Skip to content

Commit c2dab58

Browse files
meg-guptameg-gupta
committed
Update SILGen for ~Copyable borrow accessors
Introduce copy_value + mark_unresolved_non_copyable_value + begin_borrow at the return value of borrow accessor apply to drive move-only diagnostics. Also strip the copy_value + mark_unresolved_non_copyable_value + begin_borrow trio in a few places, since they create an artificial scope out of which we cannot return values in a borrow accessor without resorting to unsafe SIL operations currently. Borrow accessor diagnostics allow stripping these instructions safely in the following places: - return value of a borrow accessor - self argument reference in the borrow accessor return expression and borrow accessor apply
1 parent ab07ca9 commit c2dab58

File tree

8 files changed

+195
-11
lines changed

8 files changed

+195
-11
lines changed

‎include/swift/SIL/InstructionUtils.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ SILValue stripCastsWithoutMarkDependence(SILValue V);
4141
/// begin_borrow instructions.
4242
SILValue lookThroughOwnershipInsts(SILValue v);
4343

44+
SILValue lookThroughMoveOnlyCheckerPattern(SILValue value);
45+
4446
/// Reverse of lookThroughOwnershipInsts.
4547
///
4648
/// Return true if \p visitor returned true for all uses.

‎include/swift/SIL/SILValue.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,8 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
616616

617617
bool isBeginApplyToken() const;
618618

619+
bool isBorrowAccessorResult() const;
620+
619621
/// Unsafely eliminate moveonly from this value's type. Returns true if the
620622
/// value's underlying type was move only and thus was changed. Returns false
621623
/// otherwise.

‎lib/SIL/IR/SILValue.cpp‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -196,11 +196,7 @@ bool ValueBase::isGuaranteedForwarding() const {
196196
return phi->isGuaranteedForwarding();
197197
}
198198

199-
auto *applyInst = dyn_cast_or_null<ApplyInst>(getDefiningInstruction());
200-
if (!applyInst) {
201-
return false;
202-
}
203-
return applyInst->hasGuaranteedResult();
199+
return isBorrowAccessorResult();
204200
}
205201

206202
bool ValueBase::isBeginApplyToken() const {
@@ -210,6 +206,13 @@ bool ValueBase::isBeginApplyToken() const {
210206
return result->isBeginApplyToken();
211207
}
212208

209+
bool ValueBase::isBorrowAccessorResult() const {
210+
auto *apply = dyn_cast_or_null<ApplyInst>(getDefiningInstruction());
211+
if (!apply)
212+
return false;
213+
return apply->hasGuaranteedResult();
214+
}
215+
213216
bool ValueBase::hasDebugTrace() const {
214217
for (auto *op : getUses()) {
215218
if (auto *debugValue = dyn_cast<DebugValueInst>(op->getUser())) {

‎lib/SIL/Utils/InstructionUtils.cpp‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,22 @@ SILValue swift::lookThroughOwnershipInsts(SILValue v) {
4848
}
4949
}
5050

51+
SILValue swift::lookThroughMoveOnlyCheckerPattern(SILValue value) {
52+
auto *bbi = dyn_cast<BeginBorrowInst>(value);
53+
if (!bbi) {
54+
return value;
55+
}
56+
auto *muncvi = cast<MarkUnresolvedNonCopyableValueInst>(bbi->getOperand());
57+
if (!muncvi) {
58+
return value;
59+
}
60+
auto *cvi = cast<CopyValueInst>(muncvi->getOperand());
61+
if (!cvi) {
62+
return value;
63+
}
64+
return cvi->getOperand();
65+
}
66+
5167
bool swift::visitNonOwnershipUses(SILValue value,
5268
function_ref<bool(Operand *)> visitor) {
5369
// All ownership insts have a single operand, so a recursive walk is

‎lib/SILGen/SILGenApply.cpp‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5442,6 +5442,17 @@ ManagedValue CallEmission::applyBorrowAccessor() {
54425442
origFormalType.getLifetimeDependencies(), calleeTypeInfo.foreign,
54435443
uncurriedArgs, uncurriedLoc);
54445444

5445+
auto selfArgMV = uncurriedArgs.back();
5446+
5447+
// Strip the unnecessary copy_value + mark_unresolved_non_copyable_value +
5448+
// begin_borrow instructions added for move-only self argument.
5449+
if (selfArgMV.getValue()->getType().isMoveOnly() &&
5450+
selfArgMV.getValue()->getType().isObject()) {
5451+
uncurriedArgs[uncurriedArgs.size() - 1] =
5452+
ManagedValue::forBorrowedObjectRValue(
5453+
lookThroughMoveOnlyCheckerPattern(selfArgMV.getValue()));
5454+
}
5455+
54455456
auto value = SGF.applyBorrowAccessor(uncurriedLoc.value(), fnValue, canUnwind,
54465457
callee.getSubstitutions(), uncurriedArgs,
54475458
calleeTypeInfo.substFnType, options);
@@ -5459,13 +5470,21 @@ ManagedValue SILGenFunction::applyBorrowAccessor(
54595470
/*indirect results*/ {}, /*indirect errors*/ {}, rawResults,
54605471
ExecutorBreadcrumb());
54615472
assert(rawResults.size() == 1);
5462-
auto result = rawResults[0];
5463-
if (result->getType().isMoveOnly()) {
5464-
result = B.createMarkUnresolvedNonCopyableValueInst(
5465-
loc, result,
5473+
auto rawResult = rawResults[0];
5474+
if (!rawResult->getType().isMoveOnly()) {
5475+
return ManagedValue::forForwardedRValue(*this, rawResult);
5476+
}
5477+
if (rawResult->getType().isAddress()) {
5478+
auto result = B.createMarkUnresolvedNonCopyableValueInst(
5479+
loc, rawResult,
54665480
MarkUnresolvedNonCopyableValueInst::CheckKind::NoConsumeOrAssign);
5481+
return ManagedValue::forRValueWithoutOwnership(result);
54675482
}
5468-
return ManagedValue::forForwardedRValue(*this, result);
5483+
auto result = emitManagedCopy(loc, rawResult);
5484+
result = B.createMarkUnresolvedNonCopyableValueInst(
5485+
loc, result,
5486+
MarkUnresolvedNonCopyableValueInst::CheckKind::NoConsumeOrAssign);
5487+
return emitManagedBeginBorrow(loc, result.getValue());
54695488
}
54705489

54715490
RValue CallEmission::applyFirstLevelCallee(SGFContext C) {

‎lib/SILGen/SILGenLValue.cpp‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3171,6 +3171,27 @@ class LLVM_LIBRARY_VISIBILITY SILGenBorrowedBaseVisitor
31713171
// We are going to immediately use this base value, so we want to borrow it.
31723172
ManagedValue mv =
31733173
SGF.emitRValueAsSingleValue(e, SGFContext::AllowImmediatePlusZero);
3174+
3175+
if (forGuaranteedReturn && mv.getValue()->getType().isMoveOnly() &&
3176+
!mv.getValue()->getType().isAddress()) {
3177+
// SILGen eagerly generates copy_value +
3178+
// mark_unresolved_non_copyable_value for ~Copyable base values. The
3179+
// generated mark_unresolved_non_copyable_value instructions drive
3180+
// move-only checker diagnostics ensuring there are no consuming uses of
3181+
// this value. However, the copy_value +
3182+
// mark_unresolved_non_copyable_value pair creates an artifical scope out
3183+
// of which we cannot legally return a
3184+
// @guaranteed value.
3185+
// We have already diagosed borrow accessor returns, ensuring they return
3186+
// only projections, since projections don't create any consuming used we
3187+
// can safely look through copy_value + mark_unresolved_non_copyable_value + begin_borrow
3188+
// instructions while emitting the base value.
3189+
auto baseValue = lookThroughMoveOnlyCheckerPattern(mv.getValue());
3190+
assert(cast<SILArgument>(baseValue) ==
3191+
SGF.getFunction().getSelfArgument());
3192+
return ManagedValue::forBorrowedObjectRValue(baseValue);
3193+
}
3194+
31743195
if (mv.isPlusZeroRValueOrTrivial())
31753196
return mv;
31763197

‎lib/SILGen/SILGenStmt.cpp‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include "swift/Basic/ProfileCounter.h"
2929
#include "swift/SIL/AbstractionPatternGenerators.h"
3030
#include "swift/SIL/BasicBlockUtils.h"
31+
#include "swift/SIL/InstructionUtils.h"
3132
#include "swift/SIL/SILArgument.h"
3233
#include "swift/SIL/SILProfiler.h"
3334
#include "llvm/Support/SaveAndRestore.h"
@@ -791,7 +792,16 @@ void SILGenFunction::emitReturnExpr(SILLocation branchLoc,
791792
diag::invalid_multiple_return_borrow_accessor);
792793
return;
793794
}
794-
directResults.push_back(result->forward(*this));
795+
796+
auto resultValue = result->getValue();
797+
SILType selfType = F.getSelfArgument()->getType();
798+
if (selfType.isMoveOnly() && F.getConventions().hasGuaranteedResult()) {
799+
// If we are returning the result of borrow accessor, strip the
800+
// unnecessary copy_value + mark_unresolved_non_copyable_value
801+
// instructions.
802+
resultValue = lookThroughMoveOnlyCheckerPattern(resultValue);
803+
}
804+
directResults.push_back(resultValue);
795805
}
796806
} else {
797807
// SILValue return.

‎test/SILGen/borrow_accessor.swift‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,55 @@ public struct GenNCWrapper<T : ~Copyable> : ~Copyable {
333333
}
334334
}
335335

336+
public struct NCS: ~Copyable {
337+
var _nc: NC
338+
339+
var nc: NC {
340+
borrow {
341+
return _nc
342+
}
343+
}
344+
}
345+
346+
public struct NCWrapper: ~Copyable {
347+
var _nc: NC
348+
var _s: NCS
349+
350+
var nc: NC {
351+
borrow {
352+
return _nc
353+
}
354+
}
355+
var nested1: NC {
356+
borrow {
357+
return _s.nc
358+
}
359+
}
360+
361+
var nested2: NC {
362+
borrow {
363+
return nc
364+
}
365+
}
366+
367+
subscript(index: Int) -> NC {
368+
borrow {
369+
return _nc
370+
}
371+
}
372+
373+
var nested_subscript: NC {
374+
borrow {
375+
return self[0]
376+
}
377+
}
378+
379+
var literal: Int {
380+
borrow {
381+
return 0
382+
}
383+
}
384+
}
336385

337386
func test() {
338387
let w1 = Wrapper(_k: Klass(), _s: S(_k: Klass()))
@@ -451,3 +500,65 @@ func nctest() {
451500
// CHECK: [[REG7:%.*]] = apply [[REG6]]<T>([[REG5]], [[REG0]]) : $@convention(method) <τ_0_0> (Int, @in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_addr τ_0_0
452501
// CHECK: return [[REG7]]
453502
// CHECK: }
503+
504+
// CHECK-LABEL: sil hidden [ossa] @$s15borrow_accessor9NCWrapperV2ncAA2NCVvb : $@convention(method) (@guaranteed NCWrapper) -> @guaranteed NC {
505+
// CHECK: bb0([[REG0]] : @guaranteed $NCWrapper):
506+
// CHECK: [[REG1:%.*]] = copy_value [[REG0]]
507+
// CHECK: [[REG2:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG1]]
508+
// CHECK: [[REG4:%.*]] = begin_borrow [[REG2]]
509+
// CHECK: [[REG5:%.*]] = struct_extract [[REG0]], #NCWrapper._nc
510+
// CHECK: end_borrow [[REG4]]
511+
// CHECK: destroy_value [[REG2]]
512+
// CHECK: return [[REG5]]
513+
// CHECK: }
514+
515+
// CHECK-LABEL: sil hidden [ossa] @$s15borrow_accessor9NCWrapperV7nested1AA2NCVvb : $@convention(method) (@guaranteed NCWrapper) -> @guaranteed NC {
516+
// CHECK: bb0([[REG0]] : @guaranteed $NCWrapper):
517+
// CHECK: [[REG1:%.*]] = copy_value [[REG0]]
518+
// CHECK: [[REG2:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG1]]
519+
// CHECK: [[REG4:%.*]] = begin_borrow [[REG2]]
520+
// CHECK: [[REG5:%.*]] = struct_extract [[REG0]], #NCWrapper._s
521+
// CHECK: [[REG6:%.*]] = function_ref @$s15borrow_accessor3NCSV2ncAA2NCVvb : $@convention(method) (@guaranteed NCS) -> @guaranteed NC
522+
// CHECK: [[REG7:%.*]] = apply [[REG6]]([[REG5]]) : $@convention(method) (@guaranteed NCS) -> @guaranteed NC
523+
// CHECK: [[REG8:%.*]] = copy_value [[REG7]]
524+
// CHECK: [[REG9:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG8]]
525+
// CHECK: [[REG10:%.*]] = begin_borrow [[REG9]]
526+
// CHECK: end_borrow [[REG10]]
527+
// CHECK: destroy_value [[REG9]]
528+
// CHECK: end_borrow [[REG4]]
529+
// CHECK: destroy_value [[REG2]]
530+
// CHECK: return [[REG7]]
531+
// CHECK-LABEL: }
532+
533+
// CHECK-LABEL: sil hidden [ossa] @$s15borrow_accessor9NCWrapperVyAA2NCVSicib : $@convention(method) (Int, @guaranteed NCWrapper) -> @guaranteed NC {
534+
// CHECK: bb0([[REG0]] : $Int, [[REG1]] : @guaranteed $NCWrapper):
535+
// CHECK: [[REG3:%.*]] = copy_value [[REG1]]
536+
// CHECK: [[REG4:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG3]]
537+
// CHECK: [[REG6:%.*]] = begin_borrow [[REG4]]
538+
// CHECK: [[REG7:%.*]] = struct_extract [[REG1]], #NCWrapper._nc
539+
// CHECK: end_borrow [[REG6]]
540+
// CHECK: destroy_value [[REG4]]
541+
// CHECK: return [[REG7]]
542+
// CHECK: }
543+
544+
// CHECK-LABEL: sil hidden [ossa] @$s15borrow_accessor9NCWrapperV16nested_subscriptAA2NCVvb : $@convention(method) (@guaranteed NCWrapper) -> @guaranteed NC {
545+
// CHECK: bb0([[REG0]] : @guaranteed $NCWrapper):
546+
// CHECK: [[REG1:%.*]] = copy_value [[REG0]]
547+
// CHECK: [[REG2:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG1]]
548+
// CHECK: [[REG4:%.*]] = begin_borrow [[REG2]]
549+
// CHECK: [[REG5:%.*]] = integer_literal $Builtin.IntLiteral, 0
550+
// CHECK: [[REG6:%.*]] = metatype $@thin Int.Type
551+
// CHECK: [[REG7:%.*]] = function_ref @$sSi22_builtinIntegerLiteralSiBI_tcfC : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int
552+
// CHECK: [[REG8:%.*]] = apply [[REG7]]([[REG5]], [[REG6]]) : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int
553+
// CHECK: [[REG9:%.*]] = function_ref @$s15borrow_accessor9NCWrapperVyAA2NCVSicib : $@convention(method) (Int, @guaranteed NCWrapper) -> @guaranteed NC
554+
// CHECK: [[REG10:%.*]] = apply [[REG9]]([[REG8]], [[REG0]]) : $@convention(method) (Int, @guaranteed NCWrapper) -> @guaranteed NC
555+
// CHECK: [[REG11:%.*]] = copy_value [[REG10]]
556+
// CHECK: [[REG12:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG11]]
557+
// CHECK: [[REG13:%.*]] = begin_borrow [[REG12]]
558+
// CHECK: end_borrow [[REG13]]
559+
// CHECK: destroy_value [[REG12]]
560+
// CHECK: end_borrow [[REG4]]
561+
// CHECK: destroy_value [[REG2]]
562+
// CHECK: return [[REG10]]
563+
// CHECK: }
564+

0 commit comments

Comments
 (0)