Skip to content

Commit 0959bce

Browse files
Xazax-hunXazax-hun
authored
Merge pull request #84612 from swiftlang/fix-overrelease-destroy-hoist
[cxx-interop] Delay lowering unowned convention until ownership elimination
2 parents 7159b12 + 8b3b16c commit 0959bce

File tree

14 files changed

+115
-120
lines changed

14 files changed

+115
-120
lines changed

‎lib/SIL/IR/SILType.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,11 @@ SILResultInfo::getOwnershipKind(SILFunction &F,
655655
case ResultConvention::Owned:
656656
return OwnershipKind::Owned;
657657
case ResultConvention::Unowned:
658+
// We insert a retain right after the call returning an unowned value in
659+
// OwnershipModelEliminator. So treat the result as owned.
660+
if (IsTrivial)
661+
return OwnershipKind::None;
662+
return OwnershipKind::Owned;
658663
case ResultConvention::UnownedInnerPointer:
659664
if (IsTrivial)
660665
return OwnershipKind::None;

‎lib/SILGen/SILGenApply.cpp‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6215,13 +6215,12 @@ RValue SILGenFunction::emitApply(
62156215
B.getDefaultAtomicity());
62166216
hasAlreadyLifetimeExtendedSelf = true;
62176217
}
6218-
LLVM_FALLTHROUGH;
6219-
6220-
case ResultConvention::Unowned:
6221-
// Unretained. Retain the value.
62226218
result = resultTL.emitCopyValue(B, loc, result);
62236219
break;
62246220

6221+
case ResultConvention::Unowned:
6222+
// Handled in OwnershipModelEliminator.
6223+
break;
62256224
case ResultConvention::GuaranteedAddress:
62266225
case ResultConvention::Guaranteed:
62276226
llvm_unreachable("borrow accessor is not yet implemented");

‎lib/SILGen/SILGenPoly.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5290,6 +5290,7 @@ void ResultPlanner::execute(SmallVectorImpl<SILValue> &innerDirectResultStack,
52905290
LLVM_FALLTHROUGH;
52915291
case ResultConvention::Owned:
52925292
case ResultConvention::Autoreleased:
5293+
case ResultConvention::Unowned: // Handled in OwnershipModelEliminator.
52935294
return SGF.emitManagedRValueWithCleanup(resultValue, resultTL);
52945295
case ResultConvention::Pack:
52955296
llvm_unreachable("shouldn't have direct result with pack results");
@@ -5299,8 +5300,6 @@ void ResultPlanner::execute(SmallVectorImpl<SILValue> &innerDirectResultStack,
52995300
// originally 'self'.
53005301
SGF.SGM.diagnose(Loc.getSourceLoc(), diag::not_implemented,
53015302
"reabstraction of returns_inner_pointer function");
5302-
LLVM_FALLTHROUGH;
5303-
case ResultConvention::Unowned:
53045303
return SGF.emitManagedCopy(Loc, resultValue, resultTL);
53055304
case ResultConvention::GuaranteedAddress:
53065305
case ResultConvention::Guaranteed:

‎lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
///
2323
//===----------------------------------------------------------------------===//
2424

25+
#include "swift/AST/Types.h"
26+
#include "swift/SIL/SILValue.h"
2527
#define DEBUG_TYPE "sil-ownership-model-eliminator"
2628

2729
#include "swift/Basic/Assertions.h"
@@ -404,6 +406,20 @@ bool OwnershipModelEliminatorVisitor::visitApplyInst(ApplyInst *ai) {
404406
changed = true;
405407
}
406408

409+
// Insert a retain for unowned results.
410+
SILBuilderWithScope builder(ai->getNextInstruction(), builderCtx);
411+
auto resultIt = fnConv.getDirectSILResults().begin();
412+
auto copyValue = [&](unsigned idx, SILValue v) {
413+
auto result = *resultIt;
414+
if (result.getConvention() == ResultConvention::Unowned)
415+
builder.emitCopyValueOperation(ai->getLoc(), v);
416+
++resultIt;
417+
};
418+
if (fnConv.getNumDirectSILResults() == 1)
419+
copyValue(0, ai);
420+
else
421+
builder.emitDestructureValueOperation(ai->getLoc(), ai, copyValue);
422+
407423
return changed;
408424
}
409425

‎test/Interop/Cxx/foreign-reference/Inputs/logging-frts.h‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ class SharedFRT {
77
public:
88
SharedFRT() : _refCount(1) { logMsg("Ctor"); }
99

10+
protected:
11+
virtual ~SharedFRT() { logMsg("Dtor"); }
12+
1013
private:
1114
void logMsg(const char *s) const {
1215
printf("RefCount: %d, message: %s\n", _refCount, s);
1316
}
1417

15-
~SharedFRT() { logMsg("Dtor"); }
1618
SharedFRT(const SharedFRT &) = delete;
1719
SharedFRT &operator=(const SharedFRT &) = delete;
1820
SharedFRT(SharedFRT &&) = delete;
@@ -62,3 +64,24 @@ inline LargeStructWithRefCountedField getStruct() {
6264
inline LargeStructWithRefCountedFieldNested getNestedStruct() {
6365
return {0, {0, 0, 0, 0, new SharedFRT()}};
6466
}
67+
68+
template <class T>
69+
struct Ref {
70+
T *_Nonnull ptr() const { return ref; }
71+
T *ref;
72+
};
73+
74+
class Payload final : public SharedFRT {
75+
public:
76+
static Ref<Payload> create(int value) {
77+
Ref<Payload> ref;
78+
ref.ref = new Payload(value);
79+
return ref;
80+
}
81+
82+
int value() const { return m_value; }
83+
84+
private:
85+
explicit Payload(int value) : m_value(value) {}
86+
int m_value;
87+
};

‎test/Interop/Cxx/foreign-reference/get-frt-from-smart-ptr.swift‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// RUN: %target-run-simple-swift(-I %swift_src_root/lib/ClangImporter/SwiftBridging -I %S/Inputs -cxx-interoperability-mode=default -Xfrontend -disable-availability-checking -O) | %FileCheck %s
2+
3+
// REQUIRES: executable_test
4+
5+
import LoggingFrts
6+
7+
@inline(never)
8+
func use(_ x: CInt) {
9+
print("Value is \(x).")
10+
}
11+
12+
func testRefIssues() {
13+
var a2 = Optional.some(Payload.create(0));
14+
let b2 = a2!.ptr();
15+
a2 = Optional.none;
16+
let v2 = b2.value();
17+
use(v2)
18+
}
19+
testRefIssues()
20+
21+
// CHECK: RefCount: 1, message: Ctor
22+
// CHECK-NEXT: RefCount: 2, message: retain
23+
// CHECK-NEXT: RefCount: 1, message: release
24+
// CHECK-NEXT: Value is 0.
25+
// CHECK-NEXT: RefCount: 0, message: release
26+
// CHECK-NEXT: RefCount: 0, message: Dtor

‎test/SIL/OwnershipVerifier/use_verifier.sil‎

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -828,30 +828,6 @@ bb5(%5 : @owned $ThreeDifferingPayloadEnum):
828828
return %5 : $ThreeDifferingPayloadEnum
829829
}
830830

831-
sil [ossa] @enum_cases_with_trivial_unowned_cases_arg_into_phi : $@convention(thin) (Builtin.NativeObject) -> ThreeDifferingPayloadEnum {
832-
bb0(%0 : @unowned $Builtin.NativeObject):
833-
cond_br undef, bb1, bb2
834-
835-
bb1:
836-
cond_br undef, bb3, bb4
837-
838-
bb2:
839-
%1 = enum $ThreeDifferingPayloadEnum, #ThreeDifferingPayloadEnum.nopayload!enumelt
840-
br bb5(%1 : $ThreeDifferingPayloadEnum)
841-
842-
bb3:
843-
%2 = enum $ThreeDifferingPayloadEnum, #ThreeDifferingPayloadEnum.nontrivial_payload!enumelt, %0 : $Builtin.NativeObject
844-
br bb5(%2 : $ThreeDifferingPayloadEnum)
845-
846-
bb4:
847-
%3 = integer_literal $Builtin.Int32, 0
848-
%4 = enum $ThreeDifferingPayloadEnum, #ThreeDifferingPayloadEnum.trivial_payload!enumelt, %3 : $Builtin.Int32
849-
br bb5(%4 : $ThreeDifferingPayloadEnum)
850-
851-
bb5(%5 : @unowned $ThreeDifferingPayloadEnum):
852-
return %5 : $ThreeDifferingPayloadEnum
853-
}
854-
855831
sil [ossa] @enum_cases_with_trivial_guaranteed_cases_arg_into_phi : $@convention(thin) (@guaranteed Builtin.NativeObject) -> @owned ThreeDifferingPayloadEnum {
856832
bb0(%0 : @guaranteed $Builtin.NativeObject):
857833
cond_br undef, bb1, bb2
@@ -1383,20 +1359,6 @@ bb3(%fUnknown : @owned $@callee_owned () -> ()):
13831359
return %9999 : $()
13841360
}
13851361

1386-
sil [ossa] @unowned_to_ref_is_unowned_instant_use : $@convention(thin) (@guaranteed Builtin.NativeObject) -> Builtin.NativeObject {
1387-
bb0(%0 : @guaranteed $Builtin.NativeObject):
1388-
%1 = ref_to_unowned %0 : $Builtin.NativeObject to $@sil_unowned Builtin.NativeObject
1389-
%2 = unowned_to_ref %1 : $@sil_unowned Builtin.NativeObject to $Builtin.NativeObject
1390-
return %2 : $Builtin.NativeObject
1391-
}
1392-
1393-
sil [ossa] @unmanaged_to_ref_is_unowned_instant_use : $@convention(thin) (@guaranteed Builtin.NativeObject) -> Builtin.NativeObject {
1394-
bb0(%0 : @guaranteed $Builtin.NativeObject):
1395-
%1 = ref_to_unmanaged %0 : $Builtin.NativeObject to $@sil_unmanaged Builtin.NativeObject
1396-
%2 = unmanaged_to_ref %1 : $@sil_unmanaged Builtin.NativeObject to $Builtin.NativeObject
1397-
return %2 : $Builtin.NativeObject
1398-
}
1399-
14001362
sil [ossa] @nontrivial_enum_unchecked_enum_data_trivial_payload_owned : $@convention(thin) (@owned ThreeDifferingPayloadEnum) -> Builtin.Int32 {
14011363
bb0(%0 : @owned $ThreeDifferingPayloadEnum):
14021364
// NOTE: It may be surprising that %0 is consumed by this unchecked_enum_data

‎test/SILOptimizer/cse_objc_ossa.sil‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,13 @@ import Foundation
1414
// CHECK-NOT: objc_protocol
1515
// CHECK: tuple (%0 : $Protocol, %0 : $Protocol)
1616
// CHECK-LABEL: } // end sil function 'cse_objc_protocol'
17-
sil [ossa] @cse_objc_protocol : $@convention(thin) () -> (Protocol, Protocol) {
17+
sil [ossa] @cse_objc_protocol : $@convention(thin) () -> @owned (Protocol, Protocol) {
1818
bb0:
1919
%0 = objc_protocol #XX : $Protocol
2020
%1 = objc_protocol #XX : $Protocol
2121
%2 = tuple (%0: $Protocol, %1: $Protocol)
22-
return %2 : $(Protocol, Protocol)
22+
%3 = copy_value %2
23+
return %3
2324
}
2425

2526
@objc protocol Walkable {

‎test/SILOptimizer/cse_ossa.sil‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -700,12 +700,13 @@ bb0(%0 : $*Builtin.Int8):
700700
// CHECK-NOT: raw_pointer_to_ref
701701
// CHECK: tuple
702702
// CHECK-LABEL: } // end sil function 'cse_raw_pointer_to_ref'
703-
sil [ossa] @cse_raw_pointer_to_ref : $@convention(thin) (Builtin.RawPointer) -> (C, C) {
703+
sil [ossa] @cse_raw_pointer_to_ref : $@convention(thin) (Builtin.RawPointer) -> @owned (C, C) {
704704
bb0(%0 : $Builtin.RawPointer):
705705
%1 = raw_pointer_to_ref %0 : $Builtin.RawPointer to $C
706706
%2 = raw_pointer_to_ref %0 : $Builtin.RawPointer to $C
707707
%6 = tuple(%1: $C, %2: $C)
708-
return %6 : $(C, C)
708+
%7 = copy_value %6
709+
return %7
709710
}
710711

711712
// CHECK-LABEL: sil [ossa] @cse_unchecked_addr_cast :

‎test/SILOptimizer/cse_ossa_nontrivial.sil‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -255,19 +255,6 @@ bb0(%0 : @owned $Klass):
255255
return %6 : $()
256256
}
257257

258-
// CHECK-LABEL: sil [ossa] @cse_raw_pointer_to_ref :
259-
// CHECK: raw_pointer_to_ref
260-
// CHECK-NOT: raw_pointer_to_ref
261-
// CHECK: tuple
262-
// CHECK-LABEL: } // end sil function 'cse_raw_pointer_to_ref'
263-
sil [ossa] @cse_raw_pointer_to_ref : $@convention(thin) (Builtin.RawPointer) -> (Klass, Klass) {
264-
bb0(%0 : $Builtin.RawPointer):
265-
%1 = raw_pointer_to_ref %0 : $Builtin.RawPointer to $Klass
266-
%2 = raw_pointer_to_ref %0 : $Builtin.RawPointer to $Klass
267-
%6 = tuple(%1: $Klass, %2: $Klass)
268-
return %6 : $(Klass, Klass)
269-
}
270-
271258
enum Enum1 {
272259
case Case1
273260
case Case2

0 commit comments

Comments
 (0)