Merge pull request #24893 from gottesmm/swift-5.1-branch-rdar49903264

gottesmm · web-flow · commit 668ccd7a1f01 · 2019-05-20T20:50:53.000-07:00
[5.1][silgenpattern] When binding values, use a ManagedValue instead of an RValue to avoid implicitly exploding tuples.
diff --git a/lib/SILGen/ManagedValue.cpp b/lib/SILGen/ManagedValue.cpp
@@ -57,8 +57,8 @@ ManagedValue ManagedValue::formalAccessCopy(SILGenFunction &SGF,
 
 /// Store a copy of this value with independent ownership into the given
 /// uninitialized address.
-void ManagedValue::copyInto(SILGenFunction &SGF, SILValue dest,
-                            SILLocation loc) {
+void ManagedValue::copyInto(SILGenFunction &SGF, SILLocation loc,
+                            SILValue dest) {
   auto &lowering = SGF.getTypeLowering(getType());
   if (lowering.isAddressOnly() && SGF.silConv.useLoweredAddresses()) {
     SGF.B.createCopyAddr(loc, getValue(), dest, IsNotTake, IsInitialization);
@@ -69,6 +69,12 @@ void ManagedValue::copyInto(SILGenFunction &SGF, SILValue dest,
   lowering.emitStoreOfCopy(SGF.B, loc, copy, dest, IsInitialization);
 }
 
+void ManagedValue::copyInto(SILGenFunction &SGF, SILLocation loc,
+                            Initialization *dest) {
+  dest->copyOrInitValueInto(SGF, loc, *this, /*isInit*/ false);
+  dest->finishInitialization(SGF);
+}
+
 /// This is the same operation as 'copy', but works on +0 values that don't
 /// have cleanups.  It returns a +1 value with one.
 ManagedValue ManagedValue::copyUnmanaged(SILGenFunction &SGF, SILLocation loc) {
diff --git a/lib/SILGen/ManagedValue.h b/lib/SILGen/ManagedValue.h
@@ -268,10 +268,6 @@ class ManagedValue {
   /// formal evaluation scope.
   ManagedValue formalAccessCopy(SILGenFunction &SGF, SILLocation loc);
 
-  /// Store a copy of this value with independent ownership into the given
-  /// uninitialized address.
-  void copyInto(SILGenFunction &SGF, SILValue dest, SILLocation loc);
-
   /// This is the same operation as 'copy', but works on +0 values that don't
   /// have cleanups.  It returns a +1 value with one.
   ManagedValue copyUnmanaged(SILGenFunction &SGF, SILLocation loc);
@@ -333,7 +329,15 @@ class ManagedValue {
   /// \param loc - the AST location to associate with emitted instructions.
   /// \param address - the address to assign to.
   void assignInto(SILGenFunction &SGF, SILLocation loc, SILValue address);
-  
+
+  /// Store a copy of this value with independent ownership into the given
+  /// uninitialized address.
+  void copyInto(SILGenFunction &SGF, SILLocation loc, SILValue dest);
+
+  /// Store a copy of this value with independent ownership into the given
+  /// initialization \p dest.
+  void copyInto(SILGenFunction &SGF, SILLocation loc, Initialization *dest);
+
   explicit operator bool() const {
     // "InContext" is not considered false.
     return bool(getValue()) || valueAndFlag.getInt();
diff --git a/lib/SILGen/SILGenDecl.cpp b/lib/SILGen/SILGenDecl.cpp
@@ -205,7 +205,7 @@ copyOrInitValueIntoSingleBuffer(SILGenFunction &SGF, SILLocation loc,
     assert(value.getValue() != destAddr && "copying in place?!");
     SILValue accessAddr =
       UnenforcedFormalAccess::enter(SGF, loc, destAddr, SILAccessKind::Modify);
-    value.copyInto(SGF, accessAddr, loc);
+    value.copyInto(SGF, loc, accessAddr);
     return;
   }
   
@@ -627,7 +627,7 @@ class ReferenceStorageInitialization : public Initialization {
     if (isInit)
       value.forwardInto(SGF, loc, address);
     else
-      value.copyInto(SGF, address, loc);
+      value.copyInto(SGF, loc, address);
   }
 
   void finishUninitialized(SILGenFunction &SGF) override {
diff --git a/lib/SILGen/SILGenLValue.cpp b/lib/SILGen/SILGenLValue.cpp
@@ -1705,7 +1705,7 @@ makeBaseConsumableMaterializedRValue(SILGenFunction &SGF,
   if (!base.getType().isAddress() || isBorrowed) {
     auto tmp = SGF.emitTemporaryAllocation(loc, base.getType());
     if (isBorrowed)
-      base.copyInto(SGF, tmp, loc);
+      base.copyInto(SGF, loc, tmp);
     else
       base.forwardInto(SGF, loc, tmp);
     return SGF.emitManagedBufferWithCleanup(tmp);
diff --git a/lib/SILGen/SILGenPattern.cpp b/lib/SILGen/SILGenPattern.cpp
@@ -1193,12 +1193,11 @@ void PatternMatchEmission::bindVariable(Pattern *pattern, VarDecl *var,
 
   // Initialize the variable value.
   InitializationPtr init = SGF.emitInitializationForVarDecl(var, immutable);
-  CanType formalValueType = pattern->getType()->getCanonicalType();
-  RValue rv(SGF, pattern, formalValueType, value.getFinalManagedValue());
+  auto mv = value.getFinalManagedValue();
   if (shouldTake(value, isIrrefutable)) {
-    std::move(rv).forwardInto(SGF, pattern, init.get());
+    mv.forwardInto(SGF, pattern, init.get());
   } else {
-    std::move(rv).copyInto(SGF, pattern, init.get());
+    mv.copyInto(SGF, pattern, init.get());
   }
 }
 
diff --git a/lib/SILGen/SILGenProlog.cpp b/lib/SILGen/SILGenProlog.cpp
@@ -199,7 +199,7 @@ class EmitBBArguments : public CanTypeVisitor<EmitBBArguments,
         if (element.hasCleanup())
           element.forwardInto(SGF, loc, elementBuffer);
         else
-          element.copyInto(SGF, elementBuffer, loc);
+          element.copyInto(SGF, loc, elementBuffer);
       }
       return SGF.emitManagedRValueWithCleanup(buffer);
     }
diff --git a/test/Inputs/resilient_struct.swift b/test/Inputs/resilient_struct.swift
@@ -78,7 +78,9 @@ public struct ResilientDouble {
   }
 }
 
-public class Referent {}
+public class Referent {
+  public init() {}
+}
 
 public struct ResilientWeakRef {
   public weak var ref: Referent?
@@ -90,8 +92,21 @@ public struct ResilientWeakRef {
 
 public struct ResilientRef {
   public var r: Referent
+
+  public init(r: Referent) { self.r = r }
 }
 
 public struct ResilientWithInternalField {
   var x: Int
 }
+
+// Tuple parameters with resilient structs
+public class Subject {}
+
+public struct Container {
+  public var s: Subject
+}
+
+public struct PairContainer {
+  public var pair : (Container, Container)
+}
diff --git a/test/Interpreter/switch_objc.swift b/test/Interpreter/switch_objc.swift
@@ -0,0 +1,40 @@
+// RUN: %target-run-simple-swift
+
+// REQUIRES: executable_test
+// REQUIRES: objc_interop
+
+import StdlibUnittest
+import Foundation
+
+var SwitchTestSuite = TestSuite("SwitchObjC")
+defer { runAllTests() }
+
+SwitchTestSuite.test("Resilient Type Tuple Initialization Construction") {
+  // This test make sure that this works for URL specifically. There is a
+  // separate artificial test case in switch_resilience that uses our own
+  // resilient type.
+  enum Enum {
+  case first(url: URL, void: Void)
+  }
+
+  func getEnum() -> Enum {
+    let url = URL(string: "http://foobar.com")!
+    return .first(url: url, void: ())
+  }
+  func getBool() -> Bool { return false }
+
+  switch getEnum() {
+  case let .first(x, y) where getBool():
+    print("first")
+  case .first:
+    print("second")
+  }
+
+  switch getEnum() {
+  case let .first(value) where getBool():
+    print("third")
+  case .first:
+    print("fourth")
+  }
+  print("done")
+}
diff --git a/test/Interpreter/switch_resilience.swift b/test/Interpreter/switch_resilience.swift
@@ -0,0 +1,36 @@
+// RUN: %empty-directory(%t)
+// 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
+// RUN: %target-codesign %t/%target-library-name(resilient_struct)
+// RUN: %target-swiftc_driver -I %t -L %t %s -o %t/switch_resilience -lresilient_struct %target-rpath(%t)
+// RUN: %target-codesign %t/switch_resilience
+// RUN: %target-run %t/switch_resilience
+
+// REQUIRES: executable_test
+
+import StdlibUnittest
+import resilient_struct
+
+var SwitchResilienceTestSuite = TestSuite("SwitchResilience")
+defer { runAllTests() }
+
+enum Enum {
+case first(url: ResilientRef, void: Void)
+}
+
+func getEnum() -> Enum {
+  let url = ResilientRef(r: Referent())
+  return .first(url: url, void: ())
+}
+func getBool() -> Bool { return false }
+func urlUser(_ u: ResilientRef) {}
+func kraken() {}
+
+SwitchResilienceTestSuite.test("Resilient Type Tuple Initialization") {
+  switch getEnum() {
+  case let .first(value) where getBool():
+    urlUser(value.0)
+  case .first:
+    kraken()
+  }
+  kraken()
+}
diff --git a/test/SILGen/switch_resilience.swift b/test/SILGen/switch_resilience.swift
@@ -0,0 +1,52 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -enable-library-evolution -emit-module-path=%t/resilient_struct.swiftmodule %S/../Inputs/resilient_struct.swift
+// RUN: %target-swift-emit-silgen -I %t %s | %FileCheck %s
+
+import resilient_struct
+
+// CHECK-LABEL: sil hidden [ossa] @$s17switch_resilience29resilientTupleEltCaseEnumTestyyF : $@convention(thin) () -> () {
+// CHECK: bb0:
+// CHECK:   [[STACK_SLOT:%.*]] = alloc_stack $Enum
+//
+// CHECK: bb1:
+// CHECK:   [[VALUE:%.*]] = unchecked_take_enum_data_addr [[STACK_SLOT]] : $*Enum
+// CHECK:   [[STACK_SLOT_COPY:%.*]] = alloc_stack $(url: ResilientRef, void: ()), let, name "value"
+// CHECK:   copy_addr [[VALUE]] to [initialization] [[STACK_SLOT_COPY]]
+// CHECK:   cond_br {{%.*}}, bb2, bb3
+//
+// CHECK: bb2:
+// CHECK: destroy_addr [[STACK_SLOT_COPY]]
+// CHECK-NEXT: dealloc_stack [[STACK_SLOT_COPY]]
+// CHECK-NEXT: destroy_addr [[VALUE]]
+// CHECK-NEXT: dealloc_stack [[STACK_SLOT]]
+// CHECK-NEXT: br bb4
+//
+// CHECK: bb3:
+// CHECK-NEXT: destroy_addr [[STACK_SLOT_COPY]]
+// CHECK-NEXT: dealloc_stack [[STACK_SLOT_COPY]]
+// CHECK-NEXT: [[REPROJECT:%.*]] = tuple_element_addr [[VALUE]]
+// CHECK: destroy_addr [[REPROJECT]]
+// CHECK-NEXT: dealloc_stack [[STACK_SLOT]]
+// CHECK: br bb4
+//
+// CHECK: } // end sil function '$s17switch_resilience29resilientTupleEltCaseEnumTestyyF'
+func resilientTupleEltCaseEnumTest() {
+  enum Enum {
+  case first(url: ResilientRef, void: Void)
+  }
+
+  func getEnum() -> Enum {
+    let url = ResilientRef(r: Referent())
+    return .first(url: url, void: ())
+  }
+  func getBool() -> Bool { return false }
+  func urlUser(_ u: ResilientRef) {}
+  func kraken() {}
+
+  switch getEnum() {
+  case let .first(value) where getBool():
+    urlUser(value.0)
+  case .first:
+    kraken()
+  }
+}
diff --git a/test/SILGen/switch_var.swift b/test/SILGen/switch_var.swift
@@ -189,7 +189,7 @@ struct X : P { func p() {} }
 struct Y : P { func p() {} }
 struct Z : P { func p() {} }
 
-// CHECK-LABEL: sil hidden [ossa] @$s10switch_var05test_B2_41pyAA1P_p_tF
+// CHECK-LABEL: sil hidden [ossa] @$s10switch_var05test_B2_41pyAA1P_p_tF : $@convention(thin) (@in_guaranteed P) -> () {
 func test_var_4(p p: P) {
   // CHECK:   function_ref @$s10switch_var3fooSiyF
   switch (p, foo()) {
@@ -275,12 +275,14 @@ func test_var_4(p p: P) {
   // CHECK:   tuple_element_addr [[READ]] : {{.*}}, 1
   // CHECK:   function_ref @$s10switch_var1c1xySi_tF
   // CHECK:   destroy_value [[ZADDR]]
+  // CHECK-NEXT: destroy_addr [[PAIR]]
   // CHECK-NEXT: dealloc_stack [[PAIR]]
   // CHECK:   br [[CONT]]
     c(x: z.1)
 
   // CHECK: [[DFLT_NO_CASE3]]:
-  // CHECK:   destroy_value [[ZADDR]]
+  // CHECK-NEXT:   destroy_value [[ZADDR]]
+  // CHECK-NOT: destroy_addr
   case (_, var w):
   // CHECK:   [[PAIR_0:%.*]] = tuple_element_addr [[PAIR]] : $*(P, Int), 0
   // CHECK:   [[WADDR:%.*]] = alloc_box ${ var Int }
@@ -289,9 +291,10 @@ func test_var_4(p p: P) {
   // CHECK:   load [trivial] [[READ]]
   // CHECK:   function_ref @$s10switch_var1d1xySi_tF
   // CHECK:   destroy_value [[WADDR]]
-  // CHECK:   destroy_addr [[PAIR_0]] : $*P
-  // CHECK:   dealloc_stack [[PAIR]]
-  // CHECK:   br [[CONT]]
+  // CHECK-NEXT:   destroy_addr [[PAIR_0]] : $*P
+  // CHECK-NEXT:   dealloc_stack [[PAIR]]
+  // CHECK-NEXT:   dealloc_stack
+  // CHECK-NEXT:   br [[CONT]]
     d(x: w)
   }
   e()

Original file line number	Diff line number	Diff line change
`@@ -1193,12 +1193,11 @@ void PatternMatchEmission::bindVariable(Pattern pattern, VarDecl var,`
`1193`	`1193`
`1194`	`1194`	`// Initialize the variable value.`
`1195`	`1195`	`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();`
`1198`	`1197`	`if (shouldTake(value, isIrrefutable)) {`
`1199`		`- std::move(rv).forwardInto(SGF, pattern, init.get());`
	`1198`	`+ mv.forwardInto(SGF, pattern, init.get());`
`1200`	`1199`	`} else {`
`1201`		`- std::move(rv).copyInto(SGF, pattern, init.get());`
	`1200`	`+ mv.copyInto(SGF, pattern, init.get());`
`1202`	`1201`	`}`
`1203`	`1202`	`}`
`1204`	`1203`
Original file line number	Diff line number	Diff line change
`@@ -199,7 +199,7 @@ class EmitBBArguments : public CanTypeVisitor<EmitBBArguments,`
`199`	`199`	`if (element.hasCleanup())`
`200`	`200`	`element.forwardInto(SGF, loc, elementBuffer);`
`201`	`201`	`else`
`202`		`- element.copyInto(SGF, elementBuffer, loc);`
	`202`	`+ element.copyInto(SGF, loc, elementBuffer);`
`203`	`203`	`}`
`204`	`204`	`return SGF.emitManagedRValueWithCleanup(buffer);`
`205`	`205`	`}`