Skip to content

Commit 05845fd

Browse files
swift-ciswift-ci
authored
Merge pull request #17567 from gottesmm/pr-f41d0ac398c9ff713799f99abdfdad846fb967ab
2 parents 429a35f + 97367d3 commit 05845fd

15 files changed

+99
-26
lines changed

‎include/swift/Serialization/ModuleFormat.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ const uint16_t VERSION_MAJOR = 0;
5555
/// describe what change you made. The content of this comment isn't important;
5656
/// it just ensures a conflict if two people change the module format.
5757
/// Don't worry about adhering to the 80-column limit for this line.
58-
const uint16_t VERSION_MINOR = 421; // Last change: track whether xrefs come from Clang
58+
const uint16_t VERSION_MINOR = 422; // Last change: {strong_retain,copy}_unowned
5959

6060
using DeclIDField = BCFixed<31>;
6161

‎lib/IRGen/IRGenSIL.cpp

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -965,9 +965,7 @@ class IRGenSILFunction :
965965
void visitRetainValueInst(RetainValueInst *i);
966966
void visitRetainValueAddrInst(RetainValueAddrInst *i);
967967
void visitCopyValueInst(CopyValueInst *i);
968-
void visitCopyUnownedValueInst(CopyUnownedValueInst *i) {
969-
llvm_unreachable("unimplemented");
970-
}
968+
void visitCopyUnownedValueInst(CopyUnownedValueInst *i);
971969
void visitReleaseValueInst(ReleaseValueInst *i);
972970
void visitReleaseValueAddrInst(ReleaseValueAddrInst *i);
973971
void visitDestroyValueInst(DestroyValueInst *i);
@@ -3841,6 +3839,18 @@ visitStrongRetainUnownedInst(swift::StrongRetainUnownedInst *i) {
38413839
: irgen::Atomicity::NonAtomic);
38423840
}
38433841

3842+
void IRGenSILFunction::visitCopyUnownedValueInst(
3843+
swift::CopyUnownedValueInst *i) {
3844+
Explosion in = getLoweredExplosion(i->getOperand());
3845+
auto &ti = getReferentTypeInfo(*this, i->getOperand()->getType());
3846+
ti.strongRetainUnowned(*this, in, irgen::Atomicity::Atomic);
3847+
// Semantically we are just passing through the input parameter but as a
3848+
// strong reference... at LLVM IR level these type differences don't
3849+
// matter. So just set the lowered explosion appropriately.
3850+
Explosion output = getLoweredExplosion(i->getOperand());
3851+
setLoweredExplosion(i, output);
3852+
}
3853+
38443854
void IRGenSILFunction::visitUnownedRetainInst(swift::UnownedRetainInst *i) {
38453855
Explosion lowered = getLoweredExplosion(i->getOperand());
38463856
auto &ti = getReferentTypeInfo(*this, i->getOperand()->getType());

‎lib/SIL/SILBuilder.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,8 @@ static bool couldReduceStrongRefcount(SILInstruction *Inst) {
264264
isa<RetainValueInst>(Inst) || isa<UnownedRetainInst>(Inst) ||
265265
isa<UnownedReleaseInst>(Inst) || isa<StrongRetainUnownedInst>(Inst) ||
266266
isa<StoreWeakInst>(Inst) || isa<StrongRetainInst>(Inst) ||
267-
isa<AllocStackInst>(Inst) || isa<DeallocStackInst>(Inst))
267+
isa<AllocStackInst>(Inst) || isa<DeallocStackInst>(Inst) ||
268+
isa<CopyUnownedValueInst>(Inst))
268269
return false;
269270

270271
// Assign and copyaddr of trivial types cannot drop refcounts, and 'inits'

‎lib/SIL/SILVerifier.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1868,9 +1868,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
18681868
"Operand of unowned_retain");
18691869
require(unownedType->isLoadable(ResilienceExpansion::Maximal),
18701870
"unowned_retain requires unowned type to be loadable");
1871-
require(F.hasQualifiedOwnership(),
1872-
"copy_unowned_value is only valid in functions with qualified "
1873-
"ownership");
1871+
// *NOTE* We allow copy_unowned_value to be used throughout the entire
1872+
// pipeline even though it is a higher level instruction.
18741873
}
18751874

18761875
void checkDestroyValueInst(DestroyValueInst *I) {

‎lib/SILOptimizer/Analysis/EscapeAnalysis.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1372,6 +1372,7 @@ void EscapeAnalysis::analyzeInstruction(SILInstruction *I,
13721372
case SILInstructionKind::DeallocStackInst:
13731373
case SILInstructionKind::StrongRetainInst:
13741374
case SILInstructionKind::StrongRetainUnownedInst:
1375+
case SILInstructionKind::CopyUnownedValueInst:
13751376
case SILInstructionKind::RetainValueInst:
13761377
case SILInstructionKind::UnownedRetainInst:
13771378
case SILInstructionKind::BranchInst:

‎lib/SILOptimizer/Analysis/MemoryBehavior.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ class MemoryBehaviorVisitor
152152
}
153153
REFCOUNTINC_MEMBEHAVIOR_INST(StrongRetainInst)
154154
REFCOUNTINC_MEMBEHAVIOR_INST(StrongRetainUnownedInst)
155+
REFCOUNTINC_MEMBEHAVIOR_INST(CopyUnownedValueInst)
155156
REFCOUNTINC_MEMBEHAVIOR_INST(UnownedRetainInst)
156157
REFCOUNTINC_MEMBEHAVIOR_INST(RetainValueInst)
157158
#undef REFCOUNTINC_MEMBEHAVIOR_INST

‎lib/SILOptimizer/Analysis/SideEffectAnalysis.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,7 @@ void FunctionSideEffects::analyzeInstruction(SILInstruction *I) {
483483
return;
484484
case SILInstructionKind::StrongRetainInst:
485485
case SILInstructionKind::StrongRetainUnownedInst:
486+
case SILInstructionKind::CopyUnownedValueInst:
486487
case SILInstructionKind::RetainValueInst:
487488
case SILInstructionKind::UnownedRetainInst:
488489
getEffectsOn(I->getOperand(0))->Retains = true;

‎lib/SILOptimizer/Mandatory/GuaranteedARCOpts.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ static bool couldReduceStrongRefcount(SILInstruction *Inst) {
7575
if (isa<LoadInst>(Inst) || isa<StoreInst>(Inst) ||
7676
isa<RetainValueInst>(Inst) || isa<UnownedRetainInst>(Inst) ||
7777
isa<UnownedReleaseInst>(Inst) || isa<StrongRetainUnownedInst>(Inst) ||
78+
isa<CopyUnownedValueInst>(Inst) ||
7879
isa<StoreWeakInst>(Inst) || isa<StrongRetainInst>(Inst) ||
7980
isa<AllocStackInst>(Inst) || isa<DeallocStackInst>(Inst) ||
8081
isa<BeginAccessInst>(Inst) || isa<EndAccessInst>(Inst) ||

‎lib/SILOptimizer/Transforms/DeadStoreElimination.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ static bool isDeadStoreInertInstruction(SILInstruction *Inst) {
149149
switch (Inst->getKind()) {
150150
case SILInstructionKind::StrongRetainInst:
151151
case SILInstructionKind::StrongRetainUnownedInst:
152+
case SILInstructionKind::CopyUnownedValueInst:
152153
case SILInstructionKind::UnownedRetainInst:
153154
case SILInstructionKind::RetainValueInst:
154155
case SILInstructionKind::DeallocStackInst:

‎lib/SILOptimizer/Transforms/OwnershipModelEliminator.cpp

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ struct OwnershipModelEliminatorVisitor
5757
bool visitStoreInst(StoreInst *SI);
5858
bool visitStoreBorrowInst(StoreBorrowInst *SI);
5959
bool visitCopyValueInst(CopyValueInst *CVI);
60-
bool visitCopyUnownedValueInst(CopyUnownedValueInst *CVI);
6160
bool visitDestroyValueInst(DestroyValueInst *DVI);
6261
bool visitLoadBorrowInst(LoadBorrowInst *LBI);
6362
bool visitBeginBorrowInst(BeginBorrowInst *BBI) {
@@ -160,19 +159,6 @@ bool OwnershipModelEliminatorVisitor::visitCopyValueInst(CopyValueInst *CVI) {
160159
return true;
161160
}
162161

163-
bool OwnershipModelEliminatorVisitor::visitCopyUnownedValueInst(
164-
CopyUnownedValueInst *CVI) {
165-
B.createStrongRetainUnowned(CVI->getLoc(), CVI->getOperand(),
166-
B.getDefaultAtomicity());
167-
// Users of copy_value_unowned expect an owned value. So we need to convert
168-
// our unowned value to a ref.
169-
auto *UTRI =
170-
B.createUnownedToRef(CVI->getLoc(), CVI->getOperand(), CVI->getType());
171-
CVI->replaceAllUsesWith(UTRI);
172-
CVI->eraseFromParent();
173-
return true;
174-
}
175-
176162
bool OwnershipModelEliminatorVisitor::visitUnmanagedRetainValueInst(
177163
UnmanagedRetainValueInst *URVI) {
178164
// Now that we have set the unqualified ownership flag, destroy value

‎lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ static bool isRLEInertInstruction(SILInstruction *Inst) {
156156
case SILInstructionKind::IsUniqueInst:
157157
case SILInstructionKind::IsUniqueOrPinnedInst:
158158
case SILInstructionKind::FixLifetimeInst:
159+
case SILInstructionKind::CopyUnownedValueInst:
159160
return true;
160161
default:
161162
return false;

‎test/IRGen/copy_value_destroy_value.sil

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,22 @@ bb0(%0 : $Builtin.Int32):
3636
// CHECK: call %swift.refcounted* @swift_retain(%swift.refcounted* returned [[VAL2]])
3737
// CHECK: call void @swift_release(%swift.refcounted* [[VAL1]])
3838
// CHECK: call void @swift_release(%swift.refcounted* [[VAL2]])
39-
sil @non_trivial : $@convention(thin) (Foo) -> () {
39+
sil @non_trivial : $@convention(thin) (@guaranteed Foo) -> () {
4040
bb0(%0 : $Foo):
4141
%1 = copy_value %0 : $Foo
4242
destroy_value %1 : $Foo
4343
%2 = tuple()
4444
return %2 : $()
4545
}
46+
47+
// CHECK: define{{( protected)?}} swiftcc void @non_trivial_unowned(
48+
// CHECK: call %swift.refcounted* @swift_unownedRetainStrong(%swift.refcounted* returned %0)
49+
// CHECK: call void @swift_release(%swift.refcounted* %0)
50+
sil @non_trivial_unowned : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
51+
bb0(%0 : $Builtin.NativeObject):
52+
%1 = ref_to_unowned %0 : $Builtin.NativeObject to $@sil_unowned Builtin.NativeObject
53+
%2 = copy_unowned_value %1 : $@sil_unowned Builtin.NativeObject
54+
destroy_value %2 : $Builtin.NativeObject
55+
%9999 = tuple()
56+
return %9999 : $()
57+
}

‎test/Interpreter/strong_retain_unowned_mispairing.swift

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// RUN: %target-run-simple-opt-O-swift
2+
// REQUIRES: executable_test
3+
4+
// We were crashing here due to not preserving rc identity.
5+
// rdar://41328987
6+
7+
func takeEscaping(closure: @escaping (String) -> Void) {}
8+
9+
public class Helper {
10+
weak var o: P?
11+
12+
@_optimize(none)
13+
init(o: P) {
14+
self.o = o
15+
}
16+
}
17+
18+
protocol P: class {}
19+
20+
public class Binding: P {
21+
private var helper: Helper?
22+
23+
public init() {
24+
helper = Helper(o: self)
25+
26+
// Listen to model changes
27+
takeEscaping { [unowned self] (value: String) in
28+
self.update()
29+
}
30+
31+
takeEscaping { [unowned self] (value: String) in
32+
self.update()
33+
}
34+
}
35+
36+
func update() {}
37+
}
38+
39+
@_optimize(none)
40+
func testCrash() {
41+
_ = Binding()
42+
}
43+
44+
testCrash()

‎test/SILOptimizer/ownership_model_eliminator.sil

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,11 +114,12 @@ bb0(%0 : @owned $Builtin.NativeObject):
114114
return %9999 : $()
115115
}
116116

117+
// We no longer lower copy_unowned_value. So make sure that we actually don't.
118+
//
117119
// CHECK-LABEL: sil @copy_unowned_value_test : $@convention(thin) (@owned @sil_unowned Builtin.NativeObject) -> () {
118120
// CHECK: bb0([[ARG:%.*]] : $@sil_unowned Builtin.NativeObject):
119-
// CHECK-NEXT: strong_retain_unowned [[ARG]] : $@sil_unowned Builtin.NativeObject
120-
// CHECK-NEXT: [[OWNED_ARG:%.*]] = unowned_to_ref [[ARG]] : $@sil_unowned Builtin.NativeObject to $Builtin.NativeObject
121-
// CHECK-NEXT: strong_release [[OWNED_ARG]] : $Builtin.NativeObject
121+
// CHECK-NEXT: [[STRONG:%.*]] = copy_unowned_value [[ARG]] : $@sil_unowned Builtin.NativeObject
122+
// CHECK-NEXT: strong_release [[STRONG]] : $Builtin.NativeObject
122123
// CHECK-NEXT: unowned_release [[ARG]] : $@sil_unowned Builtin.NativeObject
123124
// CHECK-NEXT: tuple ()
124125
// CHECK-NEXT: return

‎test/lit.cfg

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -974,6 +974,18 @@ if not getattr(config, 'target_run_simple_swift', None):
974974
'%s %%t/a.out &&'
975975
'%s %%t/a.out'
976976
% (config.target_build_swift, mcp_opt, config.target_codesign, config.target_run))
977+
config.target_run_simple_opt_O_swift = (
978+
'%%empty-directory(%%t) && '
979+
'%s %s -O %%s -o %%t/a.out -module-name main && '
980+
'%s %%t/a.out &&'
981+
'%s %%t/a.out'
982+
% (config.target_build_swift, mcp_opt, config.target_codesign, config.target_run))
983+
config.target_run_simple_opt_Osize_swift = (
984+
'%%empty-directory(%%t) && '
985+
'%s %s -Osize %%s -o %%t/a.out -module-name main && '
986+
'%s %%t/a.out &&'
987+
'%s %%t/a.out'
988+
% (config.target_build_swift, mcp_opt, config.target_codesign, config.target_run))
977989
config.target_run_simple_swift_swift3 = (
978990
'%%empty-directory(%%t) && '
979991
'%s %s %%s -o %%t/a.out -module-name main -swift-version 3 && '
@@ -1097,6 +1109,8 @@ config.substitutions.append(('%target-run-simple-swiftgyb-swift3', config.target
10971109
config.substitutions.append(('%target-run-simple-swiftgyb', config.target_run_simple_swiftgyb))
10981110
config.substitutions.append(('%target-run-simple-swift-swift3', config.target_run_simple_swift_swift3))
10991111
config.substitutions.append(('%target-run-simple-swift', config.target_run_simple_swift))
1112+
config.substitutions.append(('%target-run-simple-opt-O-swift', config.target_run_simple_opt_O_swift))
1113+
config.substitutions.append(('%target-run-simple-opt-Osize-swift', config.target_run_simple_opt_Osize_swift))
11001114
config.substitutions.append(('%target-run-stdlib-swiftgyb-swift3', config.target_run_stdlib_swiftgyb_swift3))
11011115
config.substitutions.append(('%target-run-stdlib-swiftgyb', config.target_run_stdlib_swiftgyb))
11021116
config.substitutions.append(('%target-run-stdlib-swift-swift3', config.target_run_stdlib_swift_swift3))

0 commit comments

Comments
 (0)