Merge pull request #17599 from gottesmm/swift-4.2-branch-rdar41328987
[4.2][ownership] Do not lower copy_unowned_value to strong_retain_unowned.
diff --git a/include/swift/Serialization/ModuleFormat.h b/include/swift/Serialization/ModuleFormat.h
index 88cebac..0aa9263 100644
--- a/include/swift/Serialization/ModuleFormat.h
+++ b/include/swift/Serialization/ModuleFormat.h
@@ -55,7 +55,7 @@
/// describe what change you made. The content of this comment isn't important;
/// it just ensures a conflict if two people change the module format.
/// Don't worry about adhering to the 80-column limit for this line.
-const uint16_t VERSION_MINOR = 414; // Last change: track whether xrefs come from Clang
+const uint16_t VERSION_MINOR = 415; // Last change: {strong_retain,copy}_unowned
using DeclIDField = BCFixed<31>;
diff --git a/lib/IRGen/IRGenSIL.cpp b/lib/IRGen/IRGenSIL.cpp
index 3e3edf2..fefd65a 100644
--- a/lib/IRGen/IRGenSIL.cpp
+++ b/lib/IRGen/IRGenSIL.cpp
@@ -963,9 +963,7 @@
void visitRetainValueInst(RetainValueInst *i);
void visitRetainValueAddrInst(RetainValueAddrInst *i);
void visitCopyValueInst(CopyValueInst *i);
- void visitCopyUnownedValueInst(CopyUnownedValueInst *i) {
- llvm_unreachable("unimplemented");
- }
+ void visitCopyUnownedValueInst(CopyUnownedValueInst *i);
void visitReleaseValueInst(ReleaseValueInst *i);
void visitReleaseValueAddrInst(ReleaseValueAddrInst *i);
void visitDestroyValueInst(DestroyValueInst *i);
@@ -3844,6 +3842,18 @@
: irgen::Atomicity::NonAtomic);
}
+void IRGenSILFunction::visitCopyUnownedValueInst(
+ swift::CopyUnownedValueInst *i) {
+ Explosion in = getLoweredExplosion(i->getOperand());
+ auto &ti = getReferentTypeInfo(*this, i->getOperand()->getType());
+ ti.strongRetainUnowned(*this, in, irgen::Atomicity::Atomic);
+ // Semantically we are just passing through the input parameter but as a
+ // strong reference... at LLVM IR level these type differences don't
+ // matter. So just set the lowered explosion appropriately.
+ Explosion output = getLoweredExplosion(i->getOperand());
+ setLoweredExplosion(i, output);
+}
+
void IRGenSILFunction::visitUnownedRetainInst(swift::UnownedRetainInst *i) {
Explosion lowered = getLoweredExplosion(i->getOperand());
auto &ti = getReferentTypeInfo(*this, i->getOperand()->getType());
diff --git a/lib/SIL/SILBuilder.cpp b/lib/SIL/SILBuilder.cpp
index 475abd9..afa56f1 100644
--- a/lib/SIL/SILBuilder.cpp
+++ b/lib/SIL/SILBuilder.cpp
@@ -264,7 +264,8 @@
isa<RetainValueInst>(Inst) || isa<UnownedRetainInst>(Inst) ||
isa<UnownedReleaseInst>(Inst) || isa<StrongRetainUnownedInst>(Inst) ||
isa<StoreWeakInst>(Inst) || isa<StrongRetainInst>(Inst) ||
- isa<AllocStackInst>(Inst) || isa<DeallocStackInst>(Inst))
+ isa<AllocStackInst>(Inst) || isa<DeallocStackInst>(Inst) ||
+ isa<CopyUnownedValueInst>(Inst))
return false;
// Assign and copyaddr of trivial types cannot drop refcounts, and 'inits'
diff --git a/lib/SIL/SILVerifier.cpp b/lib/SIL/SILVerifier.cpp
index 2bf1250..028a933 100644
--- a/lib/SIL/SILVerifier.cpp
+++ b/lib/SIL/SILVerifier.cpp
@@ -1866,9 +1866,8 @@
"Operand of unowned_retain");
require(unownedType->isLoadable(ResilienceExpansion::Maximal),
"unowned_retain requires unowned type to be loadable");
- require(F.hasQualifiedOwnership(),
- "copy_unowned_value is only valid in functions with qualified "
- "ownership");
+ // *NOTE* We allow copy_unowned_value to be used throughout the entire
+ // pipeline even though it is a higher level instruction.
}
void checkDestroyValueInst(DestroyValueInst *I) {
diff --git a/lib/SILOptimizer/Analysis/EscapeAnalysis.cpp b/lib/SILOptimizer/Analysis/EscapeAnalysis.cpp
index 3370496..f647b02 100644
--- a/lib/SILOptimizer/Analysis/EscapeAnalysis.cpp
+++ b/lib/SILOptimizer/Analysis/EscapeAnalysis.cpp
@@ -1372,6 +1372,7 @@
case SILInstructionKind::DeallocStackInst:
case SILInstructionKind::StrongRetainInst:
case SILInstructionKind::StrongRetainUnownedInst:
+ case SILInstructionKind::CopyUnownedValueInst:
case SILInstructionKind::RetainValueInst:
case SILInstructionKind::UnownedRetainInst:
case SILInstructionKind::BranchInst:
diff --git a/lib/SILOptimizer/Analysis/MemoryBehavior.cpp b/lib/SILOptimizer/Analysis/MemoryBehavior.cpp
index 0b255a6..a0cf8ce 100644
--- a/lib/SILOptimizer/Analysis/MemoryBehavior.cpp
+++ b/lib/SILOptimizer/Analysis/MemoryBehavior.cpp
@@ -152,6 +152,7 @@
}
REFCOUNTINC_MEMBEHAVIOR_INST(StrongRetainInst)
REFCOUNTINC_MEMBEHAVIOR_INST(StrongRetainUnownedInst)
+ REFCOUNTINC_MEMBEHAVIOR_INST(CopyUnownedValueInst)
REFCOUNTINC_MEMBEHAVIOR_INST(UnownedRetainInst)
REFCOUNTINC_MEMBEHAVIOR_INST(RetainValueInst)
#undef REFCOUNTINC_MEMBEHAVIOR_INST
diff --git a/lib/SILOptimizer/Analysis/SideEffectAnalysis.cpp b/lib/SILOptimizer/Analysis/SideEffectAnalysis.cpp
index 26e83a0..d7f750e 100644
--- a/lib/SILOptimizer/Analysis/SideEffectAnalysis.cpp
+++ b/lib/SILOptimizer/Analysis/SideEffectAnalysis.cpp
@@ -481,6 +481,7 @@
return;
case SILInstructionKind::StrongRetainInst:
case SILInstructionKind::StrongRetainUnownedInst:
+ case SILInstructionKind::CopyUnownedValueInst:
case SILInstructionKind::RetainValueInst:
case SILInstructionKind::UnownedRetainInst:
getEffectsOn(I->getOperand(0))->Retains = true;
diff --git a/lib/SILOptimizer/Mandatory/GuaranteedARCOpts.cpp b/lib/SILOptimizer/Mandatory/GuaranteedARCOpts.cpp
index 1381cd1..2691259 100644
--- a/lib/SILOptimizer/Mandatory/GuaranteedARCOpts.cpp
+++ b/lib/SILOptimizer/Mandatory/GuaranteedARCOpts.cpp
@@ -75,6 +75,7 @@
if (isa<LoadInst>(Inst) || isa<StoreInst>(Inst) ||
isa<RetainValueInst>(Inst) || isa<UnownedRetainInst>(Inst) ||
isa<UnownedReleaseInst>(Inst) || isa<StrongRetainUnownedInst>(Inst) ||
+ isa<CopyUnownedValueInst>(Inst) ||
isa<StoreWeakInst>(Inst) || isa<StrongRetainInst>(Inst) ||
isa<AllocStackInst>(Inst) || isa<DeallocStackInst>(Inst) ||
isa<BeginAccessInst>(Inst) || isa<EndAccessInst>(Inst) ||
diff --git a/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp b/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp
index c59d7bf..f222139 100644
--- a/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp
+++ b/lib/SILOptimizer/Transforms/DeadStoreElimination.cpp
@@ -149,6 +149,7 @@
switch (Inst->getKind()) {
case SILInstructionKind::StrongRetainInst:
case SILInstructionKind::StrongRetainUnownedInst:
+ case SILInstructionKind::CopyUnownedValueInst:
case SILInstructionKind::UnownedRetainInst:
case SILInstructionKind::RetainValueInst:
case SILInstructionKind::DeallocStackInst:
diff --git a/lib/SILOptimizer/Transforms/OwnershipModelEliminator.cpp b/lib/SILOptimizer/Transforms/OwnershipModelEliminator.cpp
index c330ee3..a83d85f 100644
--- a/lib/SILOptimizer/Transforms/OwnershipModelEliminator.cpp
+++ b/lib/SILOptimizer/Transforms/OwnershipModelEliminator.cpp
@@ -57,7 +57,6 @@
bool visitStoreInst(StoreInst *SI);
bool visitStoreBorrowInst(StoreBorrowInst *SI);
bool visitCopyValueInst(CopyValueInst *CVI);
- bool visitCopyUnownedValueInst(CopyUnownedValueInst *CVI);
bool visitDestroyValueInst(DestroyValueInst *DVI);
bool visitLoadBorrowInst(LoadBorrowInst *LBI);
bool visitBeginBorrowInst(BeginBorrowInst *BBI) {
@@ -160,19 +159,6 @@
return true;
}
-bool OwnershipModelEliminatorVisitor::visitCopyUnownedValueInst(
- CopyUnownedValueInst *CVI) {
- B.createStrongRetainUnowned(CVI->getLoc(), CVI->getOperand(),
- B.getDefaultAtomicity());
- // Users of copy_value_unowned expect an owned value. So we need to convert
- // our unowned value to a ref.
- auto *UTRI =
- B.createUnownedToRef(CVI->getLoc(), CVI->getOperand(), CVI->getType());
- CVI->replaceAllUsesWith(UTRI);
- CVI->eraseFromParent();
- return true;
-}
-
bool OwnershipModelEliminatorVisitor::visitUnmanagedRetainValueInst(
UnmanagedRetainValueInst *URVI) {
// Now that we have set the unqualified ownership flag, destroy value
diff --git a/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp b/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp
index 8707aee..d1fa5b5 100644
--- a/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp
+++ b/lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp
@@ -156,6 +156,7 @@
case SILInstructionKind::IsUniqueInst:
case SILInstructionKind::IsUniqueOrPinnedInst:
case SILInstructionKind::FixLifetimeInst:
+ case SILInstructionKind::CopyUnownedValueInst:
return true;
default:
return false;
diff --git a/test/IRGen/copy_value_destroy_value.sil b/test/IRGen/copy_value_destroy_value.sil
index bfdc2a9..701cbf1 100644
--- a/test/IRGen/copy_value_destroy_value.sil
+++ b/test/IRGen/copy_value_destroy_value.sil
@@ -36,10 +36,22 @@
// CHECK: call %swift.refcounted* @swift_retain(%swift.refcounted* returned [[VAL2]])
// CHECK: call void @swift_release(%swift.refcounted* [[VAL1]])
// CHECK: call void @swift_release(%swift.refcounted* [[VAL2]])
-sil @non_trivial : $@convention(thin) (Foo) -> () {
+sil @non_trivial : $@convention(thin) (@guaranteed Foo) -> () {
bb0(%0 : $Foo):
%1 = copy_value %0 : $Foo
destroy_value %1 : $Foo
%2 = tuple()
return %2 : $()
}
+
+// CHECK: define{{( protected)?}} swiftcc void @non_trivial_unowned(
+// CHECK: call %swift.refcounted* @swift_unownedRetainStrong(%swift.refcounted* returned %0)
+// CHECK: call void @swift_release(%swift.refcounted* %0)
+sil @non_trivial_unowned : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
+bb0(%0 : $Builtin.NativeObject):
+ %1 = ref_to_unowned %0 : $Builtin.NativeObject to $@sil_unowned Builtin.NativeObject
+ %2 = copy_unowned_value %1 : $@sil_unowned Builtin.NativeObject
+ destroy_value %2 : $Builtin.NativeObject
+ %9999 = tuple()
+ return %9999 : $()
+}
diff --git a/test/Interpreter/strong_retain_unowned_mispairing.swift b/test/Interpreter/strong_retain_unowned_mispairing.swift
new file mode 100644
index 0000000..c701adf
--- /dev/null
+++ b/test/Interpreter/strong_retain_unowned_mispairing.swift
@@ -0,0 +1,44 @@
+// RUN: %target-run-simple-opt-O-swift
+// REQUIRES: executable_test
+
+// We were crashing here due to not preserving rc identity.
+// rdar://41328987
+
+func takeEscaping(closure: @escaping (String) -> Void) {}
+
+public class Helper {
+ weak var o: P?
+
+ @_optimize(none)
+ init(o: P) {
+ self.o = o
+ }
+}
+
+protocol P: class {}
+
+public class Binding: P {
+ private var helper: Helper?
+
+ public init() {
+ helper = Helper(o: self)
+
+ // Listen to model changes
+ takeEscaping { [unowned self] (value: String) in
+ self.update()
+ }
+
+ takeEscaping { [unowned self] (value: String) in
+ self.update()
+ }
+ }
+
+ func update() {}
+}
+
+@_optimize(none)
+func testCrash() {
+ _ = Binding()
+}
+
+testCrash()
diff --git a/test/SILOptimizer/ownership_model_eliminator.sil b/test/SILOptimizer/ownership_model_eliminator.sil
index acd3d16..6b226c6 100644
--- a/test/SILOptimizer/ownership_model_eliminator.sil
+++ b/test/SILOptimizer/ownership_model_eliminator.sil
@@ -114,11 +114,12 @@
return %9999 : $()
}
+// We no longer lower copy_unowned_value. So make sure that we actually don't.
+//
// CHECK-LABEL: sil @copy_unowned_value_test : $@convention(thin) (@owned @sil_unowned Builtin.NativeObject) -> () {
// CHECK: bb0([[ARG:%.*]] : $@sil_unowned Builtin.NativeObject):
-// CHECK-NEXT: strong_retain_unowned [[ARG]] : $@sil_unowned Builtin.NativeObject
-// CHECK-NEXT: [[OWNED_ARG:%.*]] = unowned_to_ref [[ARG]] : $@sil_unowned Builtin.NativeObject to $Builtin.NativeObject
-// CHECK-NEXT: strong_release [[OWNED_ARG]] : $Builtin.NativeObject
+// CHECK-NEXT: [[STRONG:%.*]] = copy_unowned_value [[ARG]] : $@sil_unowned Builtin.NativeObject
+// CHECK-NEXT: strong_release [[STRONG]] : $Builtin.NativeObject
// CHECK-NEXT: unowned_release [[ARG]] : $@sil_unowned Builtin.NativeObject
// CHECK-NEXT: tuple ()
// CHECK-NEXT: return
diff --git a/test/lit.cfg b/test/lit.cfg
index 7edca62..c53fad1 100644
--- a/test/lit.cfg
+++ b/test/lit.cfg
@@ -985,6 +985,16 @@
'%s %s %%s -o %%t/a.out -module-name main && '
'%s %%t/a.out'
% (config.target_build_swift, mcp_opt, config.target_run))
+ config.target_run_simple_opt_O_swift = (
+ '%%empty-directory(%%t) && '
+ '%s %s -O %%s -o %%t/a.out -module-name main && '
+ '%s %%t/a.out'
+ % (config.target_build_swift, mcp_opt, config.target_run))
+ config.target_run_simple_opt_Osize_swift = (
+ '%%empty-directory(%%t) && '
+ '%s %s -Osize %%s -o %%t/a.out -module-name main && '
+ '%s %%t/a.out'
+ % (config.target_build_swift, mcp_opt, config.target_run))
config.target_run_stdlib_swift = (
'rm -rf %%t && mkdir -p %%t && '
'%s %s %%s -o %%t/a.out -module-name main '
@@ -1065,6 +1075,8 @@
config.substitutions.append(('%target-run-simple-swiftgyb', config.target_run_simple_swiftgyb))
config.substitutions.append(('%target-run-simple-swift', config.target_run_simple_swift))
+config.substitutions.append(('%target-run-simple-opt-O-swift', config.target_run_simple_opt_O_swift))
+config.substitutions.append(('%target-run-simple-opt-Osize-swift', config.target_run_simple_opt_Osize_swift))
config.substitutions.append(('%target-run-stdlib-swiftgyb', config.target_run_stdlib_swiftgyb))
config.substitutions.append(('%target-run-stdlib-swift', config.target_run_stdlib_swift))
config.substitutions.append(('%target-repl-run-simple-swift', subst_target_repl_run_simple_swift))
