Fix SIL function side effects to handle unapplied escaping

Fixes rdar://113339972 DeadStoreElimination causes uninitialized closure context

Before this fix, the recently enabled function side effect implementation
would return no side effects for a partial apply that is not applied
in the same function. This resulted in DeadStoreElimination
incorrectly eliminating the initialization of the closure context.

The fix is to model the effects of capturing the arguments for the
closure context. The effects of running the closure body will be
considered later, at the point that the closure is
applied. Running the closure does, however, depend on the captured
values to be valid. If the value being captured is addressible,
then we need to model the effect of reading from that memory. In
this case, the capture reads from a local stack slot:

    %stack = alloc_stack $Klass
    store %ref to %stack : $*Klass
    %closure = partial_apply [callee_guaranteed] %f(%stack)
      : $@convention(thin) (@in_guaranteed Klass) -> ()

Later, when the closure is applied, we won't have any reference back
to the original stack slot. The application may not even happen in a caller.

Note that, even if the closure will be applied in the current
function, the side effects of the application are insufficient to
cover the side effects of the capture. For example, the closure
body itself may not read from an argument, but the context must
still be valid in case it is copied or if the capture itself was
not a bitwise-move.

As an optimization, we ignore the effect of captures for on-stack
partial applies. Such captures are always either a bitwise-move
or, more commonly, capture the source value by address. In these
cases, the side effects of applying the closure are sufficient to
cover the effects of the captures. And, if an on-stack closure is
not invoked in the current function (or passed to a callee) then
it will never be invoked, so the captures never have effects.

Author: atrick

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeSideEffects.swift

@@ -142,6 +142,24 @@ private struct CollectedEffects {
         handleApply(pa)
         checkedIfDeinitBarrier = true
       }
+      // In addition to the effects of the apply, also consider the
+      // effects of the capture, which reads the captured value in
+      // order to move it into the context. This only applies to
+      // addressible values, because capturing does not dereference
+      // any class objects.
+      //
+      // Ignore captures for on-stack partial applies. They only
+      // bitwise-move or capture by address, so the call to
+      // handleApply above is sufficient. And, if they are not applied
+      // in this function, then they are never applied.
+      if !pa.isOnStack {
+        // the callee and its arguments are all captured...
+        for operand in pa.operands {
+          if operand.value.type.isAddress {
+            addEffects(.read, to: operand.value)
+          }
+        }
+      }
 
     case let fl as FixLifetimeInst:
       // A fix_lifetime instruction acts like a read on the operand to prevent

test/SILOptimizer/dead_store_elim.sil

@@ -1,4 +1,5 @@
 // RUN: %target-sil-opt %s -dead-store-elimination -enable-sil-verify-all | %FileCheck %s
+// RUN: %target-sil-opt %s -compute-side-effects -dead-store-elimination -enable-sil-verify-all | %FileCheck %s --check-prefix=CHECK-SEA-DSE
 
 // REQUIRES: swift_in_compiler
 
@@ -1631,3 +1632,31 @@ bb0(%0 : $UInt64, %1 : $Builtin.Word):
   return %76 : $Builtin.Int8
 }
 
+class Klass {}
+
+sil @klassClosure : $@convention(thin) (@in_guaranteed Klass) -> ()
+
+sil @test_pa_without_apply : $@convention(thin) (@in Klass) -> @callee_guaranteed () -> () {
+bb0(%0 : $*Klass):
+  %3 = function_ref @klassClosure : $@convention(thin) (@in_guaranteed Klass) -> ()
+  %4 = partial_apply [callee_guaranteed] %3(%0) : $@convention(thin) (@in_guaranteed Klass) -> ()
+  return %4 : $@callee_guaranteed ()  -> ()
+}
+
+// CHECK-SEA-DSE: sil @dont_dead_store_capture :
+// CHECK-SEA-DSE: store
+// CHECK-SEA-DSE: } // end sil function 'dont_dead_store_capture'
+sil @dont_dead_store_capture : $@convention(thin) (@in_guaranteed (Klass, Klass)) -> () {
+bb0(%0 : $*(Klass, Klass)):
+  %ele = tuple_element_addr %0 : $*(Klass, Klass), 1
+  %1 = load %ele : $*Klass
+  %3 = alloc_stack $Klass
+  store %1 to %3 : $*Klass
+  strong_retain %1 : $Klass
+  %4 = function_ref @test_pa_without_apply : $@convention(thin) (@in Klass) -> @callee_guaranteed () -> ()
+  %5 = apply %4(%3) : $@convention(thin) (@in Klass) -> @callee_guaranteed () -> ()
+  %6 = apply %5() : $@callee_guaranteed () -> ()
+  dealloc_stack %3 : $*Klass
+  %7 = tuple ()
+  return %7 : $()
+}

test/SILOptimizer/side_effects.sil

@@ -138,7 +138,7 @@ bb0(%0 : $X, %1 : $*Int32):
 
 // CHECK-LABEL: sil @partial_apply_load_store_to_args
 // CHECK-NEXT:  [%0: read v**]
-// CHECK-NEXT:  [%1: write v**]
+// CHECK-NEXT:  [%1: read v**, write v**]
 // CHECK-NEXT:  [%2: write c0.v**, destroy c*.v**]
 // CHECK-NEXT:  [global: read,write,copy,destroy,allocate,deinit_barrier]
 // CHECK-NEXT:  {{^[^[]}}
@@ -157,7 +157,7 @@ bb0(%0 : $*Int32, %1 : $*Int32, %2 : $X):
 
 // CHECK-LABEL: sil @guaranteed_partial_apply_load_store_to_args
 // CHECK-NEXT:  [%0: read v**]
-// CHECK-NEXT:  [%1: write v**]
+// CHECK-NEXT:  [%1: read v**, write v**]
 // CHECK-NEXT:  [%2: write c0.v**]
 // CHECK-NEXT:  [global: ]
 // CHECK-NEXT:  {{^[^[]}}
@@ -827,6 +827,7 @@ bb0(%0 : $X):
 }
 
 // CHECK-LABEL: sil @not_called_partial_apply
+// CHECK-NEXT:  [%0: read v**]
 // CHECK-NEXT:  [global: ]
 // CHECK-NEXT:  {{^[^[]}}
 sil @not_called_partial_apply : $@convention(thin) (@in Int32) ->  @owned @callee_owned (Bool) -> Int32 {
@@ -837,6 +838,8 @@ bb0(%0 : $*Int32):
 }
 
 // CHECK-LABEL: sil @partial_apply_chain
+// CHECK-NEXT: [%0: read v**]
+// CHECK-NEXT: [%1: read v**]
 // CHECK-NEXT: [global: ]
 // CHECK-NEXT: {{^[^[]}}
 sil @partial_apply_chain : $@convention(thin) (@in Int32, @in Int32) ->  @owned @callee_owned (Bool) -> Int32 {
@@ -849,7 +852,7 @@ bb0(%0 : $*Int32, %1 : $*Int32):
 
 // CHECK-LABEL: sil @two_nonstack_partial_applies
 // CHECK-NEXT:  [%0: read v**]
-// CHECK-NEXT:  [%1: write v**]
+// CHECK-NEXT:  [%1: read v**, write v**]
 // CHECK-NEXT:  [%2: write c0.v**]
 // CHECK-NEXT:  [global: ]
 // CHECK-NEXT:  {{^[^[]}}