[move-only] Fix SILOptimizer to avoid removing struct deinits.

Many basic SIL passes were silently deleting the struct deinit:
- SROA
- Mem2Reg
- RLE
- DSE
- FunctionSignatureOpts
- LowerAggregates

Fixes rdar://109849028 ([move-only] LowerAggregateInstrs eliminates struct deinitialization)

Author: atrick

include/swift/SILOptimizer/Utils/InstOptUtils.h

@@ -381,8 +381,10 @@ ignore_expect_uses(ValueBase *value) {
 /// operations from it. These can be simplified and removed.
 bool simplifyUsers(SingleValueInstruction *inst);
 
-///  True if a type can be expanded
-/// without a significant increase to code size.
+/// True if a type can be expanded without a significant increase to code size.
+///
+/// False if expanding a type is invalid. For example, expanding a
+/// struct-with-deinit drops the deinit.
 bool shouldExpand(SILModule &module, SILType ty);
 
 /// Check if the value of value is computed by means of a simple initialization.

lib/SILOptimizer/Transforms/SILLowerAggregateInstrs.cpp

@@ -36,9 +36,6 @@ using namespace swift::Lowering;
 
 STATISTIC(NumExpand, "Number of instructions expanded");
 
-static llvm::cl::opt<bool> EnableExpandAll("sil-lower-agg-instrs-expand-all",
-                                           llvm::cl::init(false));
-
 //===----------------------------------------------------------------------===//
 //                                  Utility
 //===----------------------------------------------------------------------===//
@@ -48,9 +45,11 @@ static llvm::cl::opt<bool> EnableExpandAll("sil-lower-agg-instrs-expand-all",
 /// optimizer. To the high level ARC optimizer, this is just noise and
 /// unnecessary IR. At the same time for testing purposes, we want to provide a
 /// way even with ownership enabled to expand so we can check correctness.
+///
+/// shouldExpand returns false for struct-with-deinit types, so bypassing it is
+/// incorrect.
 static bool shouldExpandShim(SILFunction *fn, SILType type) {
-  return EnableExpandAll ||
-         (!fn->hasOwnership() && shouldExpand(fn->getModule(), type));
+  return !fn->hasOwnership() && shouldExpand(fn->getModule(), type);
 }
 
 //===----------------------------------------------------------------------===//

lib/SILOptimizer/Utils/InstOptUtils.cpp

@@ -1120,14 +1120,24 @@ bool swift::simplifyUsers(SingleValueInstruction *inst) {
   return changed;
 }
 
-/// True if a type can be expanded without a significant increase to code size.
+// True if a type can be expanded without a significant increase to code size.
+//
+// False if expanding a type is invalid. For example, expanding a
+// struct-with-deinit drops the deinit.
 bool swift::shouldExpand(SILModule &module, SILType ty) {
   // FIXME: Expansion
   auto expansion = TypeExpansionContext::minimal();
 
   if (module.Types.getTypeLowering(ty, expansion).isAddressOnly()) {
     return false;
   }
+  // A move-only-with-deinit type cannot be SROA.
+  //
+  // TODO: we could loosen this requirement if all paths lead to a drop_deinit.
+  if (auto *nominalTy = ty.getNominalOrBoundGenericNominal()) {
+    if (nominalTy->getValueTypeDestructor())
+      return false;
+  }
   if (EnableExpandAll) {
     return true;
   }

test/SILOptimizer/loweraggregateinstrs_moveonly.sil

@@ -0,0 +1,32 @@
+// RUN: %target-sil-opt -enable-sil-verify-all -lower-aggregate-instrs %s | %FileCheck %s
+
+sil_stage canonical
+
+import Builtin
+
+struct S : ~Copyable {
+  deinit {}
+}
+struct S2 : ~Copyable {
+  var s1 = S()
+  var s2 = S()
+}
+
+// Test that a struct-with-deinit is not expanded. Doing so would
+// forget the deinit.
+//
+// public func testDeinitTwo() {
+//   var s = S2()
+// }
+//
+// CHECK-LABEL: sil @testDeinitTwo : $@convention(thin) () -> () {
+// CHECK:         release_value %{{.*}} : $S2
+// CHECK-LABEL: } // end sil function 'testDeinitTwo'
+sil @testDeinitTwo : $@convention(thin) () -> () {
+bb0:
+  %0 = struct $S ()
+  %1 = struct $S2 (%0 : $S, %0 : $S)
+  release_value %1 : $S2
+  %5 = tuple ()
+  return %5 : $()
+}