Skip to content

Commit adcfda6

Browse files
gottesmmgottesmm
committed
[silgenpattern] When binding values, use a ManagedValue instead of an RValue to avoid implicitly exploding tuples.
In SILGenPattern, we need to be able to unforward cleanups when we explode tuples. Thus we can't use RValue in SILGenPattern since it may implicitly explode tuples (which without modifying RValue itself we can not unforward). This patch removes a specific RValue usage that we can replace with the use of a ManagedValue instead. rdar://49903264
1 parent 60cd149 commit adcfda6

File tree

8 files changed

+154
-11
lines changed

8 files changed

+154
-11
lines changed

‎lib/SILGen/ManagedValue.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,12 @@ void ManagedValue::copyInto(SILGenFunction &SGF, SILLocation loc,
6969
lowering.emitStoreOfCopy(SGF.B, loc, copy, dest, IsInitialization);
7070
}
7171

72+
void ManagedValue::copyInto(SILGenFunction &SGF, SILLocation loc,
73+
Initialization *dest) {
74+
dest->copyOrInitValueInto(SGF, loc, *this, /*isInit*/ false);
75+
dest->finishInitialization(SGF);
76+
}
77+
7278
/// This is the same operation as 'copy', but works on +0 values that don't
7379
/// have cleanups. It returns a +1 value with one.
7480
ManagedValue ManagedValue::copyUnmanaged(SILGenFunction &SGF, SILLocation loc) {

‎lib/SILGen/ManagedValue.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,10 @@ class ManagedValue {
334334
/// uninitialized address.
335335
void copyInto(SILGenFunction &SGF, SILLocation loc, SILValue dest);
336336

337+
/// Store a copy of this value with independent ownership into the given
338+
/// initialization \p dest.
339+
void copyInto(SILGenFunction &SGF, SILLocation loc, Initialization *dest);
340+
337341
explicit operator bool() const {
338342
// "InContext" is not considered false.
339343
return bool(getValue()) || valueAndFlag.getInt();

‎lib/SILGen/SILGenPattern.cpp‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1193,12 +1193,11 @@ void PatternMatchEmission::bindVariable(Pattern *pattern, VarDecl *var,
11931193

11941194
// Initialize the variable value.
11951195
InitializationPtr init = SGF.emitInitializationForVarDecl(var, immutable);
1196-
CanType formalValueType = pattern->getType()->getCanonicalType();
1197-
RValue rv(SGF, pattern, formalValueType, value.getFinalManagedValue());
1196+
auto mv = value.getFinalManagedValue();
11981197
if (shouldTake(value, isIrrefutable)) {
1199-
std::move(rv).forwardInto(SGF, pattern, init.get());
1198+
mv.forwardInto(SGF, pattern, init.get());
12001199
} else {
1201-
std::move(rv).copyInto(SGF, pattern, init.get());
1200+
mv.copyInto(SGF, pattern, init.get());
12021201
}
12031202
}
12041203

‎test/Inputs/resilient_struct.swift‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,9 @@ public struct ResilientDouble {
7878
}
7979
}
8080

81-
public class Referent {}
81+
public class Referent {
82+
public init() {}
83+
}
8284

8385
public struct ResilientWeakRef {
8486
public weak var ref: Referent?
@@ -90,6 +92,8 @@ public struct ResilientWeakRef {
9092

9193
public struct ResilientRef {
9294
public var r: Referent
95+
96+
public init(r: Referent) { self.r = r }
9397
}
9498

9599
public struct ResilientWithInternalField {
@@ -105,4 +109,4 @@ public struct Container {
105109

106110
public struct PairContainer {
107111
public var pair : (Container, Container)
108-
}
112+
}

‎test/Interpreter/switch_objc.swift‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// RUN: %target-run-simple-swift
2+
3+
// REQUIRES: executable_test
4+
// REQUIRES: objc_interop
5+
6+
import StdlibUnittest
7+
import Foundation
8+
9+
var SwitchTestSuite = TestSuite("SwitchObjC")
10+
defer { runAllTests() }
11+
12+
SwitchTestSuite.test("Resilient Type Tuple Initialization Construction") {
13+
// This test make sure that this works for URL specifically. There is a
14+
// separate artificial test case in switch_resilience that uses our own
15+
// resilient type.
16+
enum Enum {
17+
case first(url: URL, void: Void)
18+
}
19+
20+
func getEnum() -> Enum {
21+
let url = URL(string: "http://foobar.com")!
22+
return .first(url: url, void: ())
23+
}
24+
func getBool() -> Bool { return false }
25+
26+
switch getEnum() {
27+
case let .first(x, y) where getBool():
28+
print("first")
29+
case .first:
30+
print("second")
31+
}
32+
33+
switch getEnum() {
34+
case let .first(value) where getBool():
35+
print("third")
36+
case .first:
37+
print("fourth")
38+
}
39+
print("done")
40+
}

‎test/Interpreter/switch_resilience.swift‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift-dylib(%t/%target-library-name(resilient_struct)) %S/../Inputs/resilient_struct.swift -emit-module -emit-module-path %t/resilient_struct.swiftmodule -module-name resilient_struct -I%t -L%t -enable-library-evolution
3+
// RUN: %target-swiftc_driver -I %t -L %t %s -o %t/switch_resilience -lresilient_struct %target-rpath(%t)
4+
// RUN: %target-codesign %t/switch_resilience
5+
// RUN: %target-run %t/switch_resilience
6+
7+
// REQUIRES: executable_test
8+
9+
import StdlibUnittest
10+
import resilient_struct
11+
12+
var SwitchResilienceTestSuite = TestSuite("SwitchResilience")
13+
defer { runAllTests() }
14+
15+
enum Enum {
16+
case first(url: ResilientRef, void: Void)
17+
}
18+
19+
func getEnum() -> Enum {
20+
let url = ResilientRef(r: Referent())
21+
return .first(url: url, void: ())
22+
}
23+
func getBool() -> Bool { return false }
24+
func urlUser(_ u: ResilientRef) {}
25+
func kraken() {}
26+
27+
SwitchResilienceTestSuite.test("Resilient Type Tuple Initialization") {
28+
switch getEnum() {
29+
case let .first(value) where getBool():
30+
urlUser(value.0)
31+
case .first:
32+
kraken()
33+
}
34+
kraken()
35+
}

‎test/SILGen/switch_resilience.swift‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend -emit-module -enable-library-evolution -emit-module-path=%t/resilient_struct.swiftmodule %S/../Inputs/resilient_struct.swift
3+
// RUN: %target-swift-emit-silgen -I %t %s | %FileCheck %s
4+
5+
import resilient_struct
6+
7+
// CHECK-LABEL: sil hidden [ossa] @$s17switch_resilience29resilientTupleEltCaseEnumTestyyF : $@convention(thin) () -> () {
8+
// CHECK: bb0:
9+
// CHECK: [[STACK_SLOT:%.*]] = alloc_stack $Enum
10+
//
11+
// CHECK: bb1:
12+
// CHECK: [[VALUE:%.*]] = unchecked_take_enum_data_addr [[STACK_SLOT]] : $*Enum
13+
// CHECK: [[STACK_SLOT_COPY:%.*]] = alloc_stack $(url: ResilientRef, void: ()), let, name "value"
14+
// CHECK: copy_addr [[VALUE]] to [initialization] [[STACK_SLOT_COPY]]
15+
// CHECK: cond_br {{%.*}}, bb2, bb3
16+
//
17+
// CHECK: bb2:
18+
// CHECK: destroy_addr [[STACK_SLOT_COPY]]
19+
// CHECK-NEXT: dealloc_stack [[STACK_SLOT_COPY]]
20+
// CHECK-NEXT: destroy_addr [[VALUE]]
21+
// CHECK-NEXT: dealloc_stack [[STACK_SLOT]]
22+
// CHECK-NEXT: br bb4
23+
//
24+
// CHECK: bb3:
25+
// CHECK-NEXT: destroy_addr [[STACK_SLOT_COPY]]
26+
// CHECK-NEXT: dealloc_stack [[STACK_SLOT_COPY]]
27+
// CHECK-NEXT: [[REPROJECT:%.*]] = tuple_element_addr [[VALUE]]
28+
// CHECK: destroy_addr [[REPROJECT]]
29+
// CHECK-NEXT: dealloc_stack [[STACK_SLOT]]
30+
// CHECK: br bb4
31+
//
32+
// CHECK: } // end sil function '$s17switch_resilience29resilientTupleEltCaseEnumTestyyF'
33+
func resilientTupleEltCaseEnumTest() {
34+
enum Enum {
35+
case first(url: ResilientRef, void: Void)
36+
}
37+
38+
func getEnum() -> Enum {
39+
let url = ResilientRef(r: Referent())
40+
return .first(url: url, void: ())
41+
}
42+
func getBool() -> Bool { return false }
43+
func urlUser(_ u: ResilientRef) {}
44+
func kraken() {}
45+
46+
switch getEnum() {
47+
case let .first(value) where getBool():
48+
urlUser(value.0)
49+
case .first:
50+
kraken()
51+
}
52+
}

‎test/SILGen/switch_var.swift‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ struct X : P { func p() {} }
189189
struct Y : P { func p() {} }
190190
struct Z : P { func p() {} }
191191

192-
// CHECK-LABEL: sil hidden [ossa] @$s10switch_var05test_B2_41pyAA1P_p_tF
192+
// CHECK-LABEL: sil hidden [ossa] @$s10switch_var05test_B2_41pyAA1P_p_tF : $@convention(thin) (@in_guaranteed P) -> () {
193193
func test_var_4(p p: P) {
194194
// CHECK: function_ref @$s10switch_var3fooSiyF
195195
switch (p, foo()) {
@@ -275,12 +275,14 @@ func test_var_4(p p: P) {
275275
// CHECK: tuple_element_addr [[READ]] : {{.*}}, 1
276276
// CHECK: function_ref @$s10switch_var1c1xySi_tF
277277
// CHECK: destroy_value [[ZADDR]]
278+
// CHECK-NEXT: destroy_addr [[PAIR]]
278279
// CHECK-NEXT: dealloc_stack [[PAIR]]
279280
// CHECK: br [[CONT]]
280281
c(x: z.1)
281282

282283
// CHECK: [[DFLT_NO_CASE3]]:
283-
// CHECK: destroy_value [[ZADDR]]
284+
// CHECK-NEXT: destroy_value [[ZADDR]]
285+
// CHECK-NOT: destroy_addr
284286
case (_, var w):
285287
// CHECK: [[PAIR_0:%.*]] = tuple_element_addr [[PAIR]] : $*(P, Int), 0
286288
// CHECK: [[WADDR:%.*]] = alloc_box ${ var Int }
@@ -289,9 +291,10 @@ func test_var_4(p p: P) {
289291
// CHECK: load [trivial] [[READ]]
290292
// CHECK: function_ref @$s10switch_var1d1xySi_tF
291293
// CHECK: destroy_value [[WADDR]]
292-
// CHECK: destroy_addr [[PAIR_0]] : $*P
293-
// CHECK: dealloc_stack [[PAIR]]
294-
// CHECK: br [[CONT]]
294+
// CHECK-NEXT: destroy_addr [[PAIR_0]] : $*P
295+
// CHECK-NEXT: dealloc_stack [[PAIR]]
296+
// CHECK-NEXT: dealloc_stack
297+
// CHECK-NEXT: br [[CONT]]
295298
d(x: w)
296299
}
297300
e()

0 commit comments

Comments
 (0)