[pmo] Eliminate PartialStore and consider Assign to be something that stops

allocations from being removed.

PartialStore and Assign in PMO today both are allowed and do not prevent an
allocation from being removed. This is incorrect with ownership since in both
cases if we have a non-trivial type, we would need to make sure that the value
being overwritten is destroyed.

Clearly, with ownership given that we do not know how to handle assigns in the
aforementioned way, we can not remove allocation with ownership. When I make the
optimization more conservative by changing assigns to stop allocations from
being removed, I see no difference in the tests/benchmarks/etc. So it makes
sense to make the change now.

The reason to remove PartialStore is that:

1. It is actually a vestigal remnant of Definite Init code in Predictable Mem
Opts. If you look in Definite Init, understanding PartialStores is a very
important part of the algorithm for diagnosing partially initialized memory
locations. In Predictable Mem Opts it only controlled whether or not we could
eliminate an allocation.

2. PartialStore assumes that values are always initialized completely so a
partial store must be an assign. Despite that the two places it was used in the
code was a place where we assumed that SILGen was always going to always have an
init anyways (meaning we produced a partial store without need) or in a case
where if we do not have an init, we can just treat it like an assign (the
copy_addr) case.

Author: gottesmm

lib/SILOptimizer/Mandatory/PMOMemoryUseCollector.cpp

@@ -254,14 +254,11 @@ bool ElementUseCollector::collectUses(SILValue Pointer) {
 
       // Coming out of SILGen, we assume that raw stores are initializations,
       // unless they have trivial type (which we classify as InitOrAssign).
-      PMOUseKind Kind;
-      if (InStructSubElement)
-        Kind = PMOUseKind::PartialStore;
-      else if (PointeeType.isTrivial(User->getModule()))
-        Kind = PMOUseKind::InitOrAssign;
-      else
-        Kind = PMOUseKind::Initialization;
-
+      auto Kind = ([&]() -> PMOUseKind {
+        if (PointeeType.isTrivial(User->getModule()))
+          return PMOUseKind::InitOrAssign;
+        return PMOUseKind::Initialization;
+      })();
       Uses.emplace_back(User, Kind);
       continue;
     }
@@ -270,9 +267,7 @@ bool ElementUseCollector::collectUses(SILValue Pointer) {
   if (auto *SI = dyn_cast<Store##Name##Inst>(User)) {                          \
     if (UI->getOperandNumber() == 1) {                                         \
       PMOUseKind Kind;                                                         \
-      if (InStructSubElement)                                                  \
-        Kind = PMOUseKind::PartialStore;                                       \
-      else if (SI->isInitializationOfDest())                                   \
+      if (SI->isInitializationOfDest())                                        \
         Kind = PMOUseKind::Initialization;                                     \
       else                                                                     \
         Kind = PMOUseKind::Assign;                                             \
@@ -294,16 +289,17 @@ bool ElementUseCollector::collectUses(SILValue Pointer) {
       // the destination, then this is an unknown assignment.  Note that we'll
       // revisit this instruction and add it to Uses twice if it is both a load
       // and store to the same aggregate.
-      PMOUseKind Kind;
-      if (UI->getOperandNumber() == 0)
-        Kind = PMOUseKind::Load;
-      else if (InStructSubElement)
-        Kind = PMOUseKind::PartialStore;
-      else if (CAI->isInitializationOfDest())
-        Kind = PMOUseKind::Initialization;
-      else
-        Kind = PMOUseKind::Assign;
-
+      //
+      // Inline constructor.
+      auto Kind = ([&]() -> PMOUseKind {
+        if (UI->getOperandNumber() == CopyAddrInst::Src)
+          return PMOUseKind::Load;
+        if (PointeeType.isTrivial(CAI->getModule()))
+          return PMOUseKind::InitOrAssign;
+        if (CAI->isInitializationOfDest())
+          return PMOUseKind::Initialization;
+        return PMOUseKind::Assign;
+      })();
       Uses.emplace_back(User, Kind);
       continue;
     }

lib/SILOptimizer/Mandatory/PMOMemoryUseCollector.h

@@ -116,9 +116,6 @@ enum PMOUseKind {
   /// value.
   Assign,
 
-  /// The instruction is a store to a member of a larger struct value.
-  PartialStore,
-
   /// An indirect 'inout' parameter of an Apply instruction.
   InOutUse,
 

lib/SILOptimizer/Mandatory/PredictableMemOpt.cpp

@@ -911,7 +911,7 @@ void AvailableValueDataflowContext::explodeCopyAddr(CopyAddrInst *CAI) {
   assert((LoadUse.isValid() || StoreUse.isValid()) &&
          "we should have a load or a store, possibly both");
   assert(StoreUse.isInvalid() || StoreUse.Kind == Assign ||
-         StoreUse.Kind == PartialStore || StoreUse.Kind == Initialization);
+         StoreUse.Kind == Initialization || StoreUse.Kind == InitOrAssign);
 
   // Now that we've emitted a bunch of instructions, including a load and store
   // but also including other stuff, update the internal state of
@@ -1286,7 +1286,7 @@ bool AllocOptimize::tryToRemoveDeadAllocation() {
 
     switch (u.Kind) {
     case PMOUseKind::Assign:
-    case PMOUseKind::PartialStore:
+      return false;
     case PMOUseKind::InitOrAssign:
       break;    // These don't prevent removal.
     case PMOUseKind::Initialization:

test/SILOptimizer/predictable_memopt.sil

@@ -24,11 +24,12 @@ bb0(%0 : $Int):
 // Verify that promotion has promoted the tuple load away, and we know that
 // %0 is being returned through scalar instructions in SSA form.
 //
-// CHECK-LABEL: sil @tuple_reg_promotion
+// CHECK-LABEL: sil @tuple_reg_promotion :
 // CHECK: bb0(%0 : $Int):
 // CHECK-NEXT: [[TUPLE:%[0-9]+]] = tuple ({{.*}} : $Int, {{.*}} : $Int)
 // CHECK-NEXT: [[TUPLE_ELT:%[0-9]+]] = tuple_extract [[TUPLE]] : $(Int, Int), 0
 // CHECK-NEXT: return [[TUPLE_ELT]] : $Int
+// CHECK-NEXT: } // end sil function 'tuple_reg_promotion'
 sil @tuple_reg_promotion : $@convention(thin) (Int) -> Int {
 bb0(%0 : $Int):                         
   %1 = alloc_box $<τ_0_0> { var τ_0_0 } <(Int, Int)>
@@ -290,10 +291,14 @@ bb0(%0 : $Int32):
   // CHECK-NEXT: return [[IL]]
   return %25 : $Builtin.Int32
 }
+// CHECK: } // end sil function 'promote_alloc_stack'
 
-// CHECK-LABEL: sil @copy_addr_to_load
+// CHECK-LABEL: sil @copy_addr_to_load : $@convention(thin) (Int) -> Int {
+// CHECK: bb0(%0 : $Int):
+// CHECK-NEXT: return %0
+// CHECK-NEXT: } // end sil function 'copy_addr_to_load'
 sil @copy_addr_to_load : $@convention(thin) (Int) -> Int {
-bb0(%0 : $Int):               // CHECK: bb0(%0 : $Int):
+bb0(%0 : $Int):
   %1 = alloc_stack $Int
   store %0 to %1 : $*Int
   %2 = alloc_stack $Int
@@ -304,14 +309,16 @@ bb0(%0 : $Int):               // CHECK: bb0(%0 : $Int):
 
   dealloc_stack %2 : $*Int
   dealloc_stack %1 : $*Int
-  // CHECK-NEXT: return %0
   return %3 : $Int
 }
 
 // rdar://15170149
-// CHECK-LABEL: sil @store_to_copyaddr
-sil @store_to_copyaddr : $(Bool) -> Bool {
-bb0(%0 : $Bool):  // CHECK: bb0(%0 :
+// CHECK-LABEL: sil @store_to_copyaddr : $@convention(thin) (Bool) -> Bool {
+// CHECK: bb0([[ARG:%.*]] :
+// CHECK-NEXT: return [[ARG]]
+// CHECK-NEXT: } // end sil function 'store_to_copyaddr'
+sil @store_to_copyaddr : $@convention(thin) (Bool) -> Bool {
+bb0(%0 : $Bool):
   %1 = alloc_stack $Bool
   store %0 to %1 : $*Bool
   %3 = alloc_stack $Bool
@@ -321,7 +328,7 @@ bb0(%0 : $Bool):  // CHECK: bb0(%0 :
   %12 = load %1 : $*Bool
   dealloc_stack %3 : $*Bool
   dealloc_stack %1 : $*Bool
-  return %12 : $Bool                              // CHECK-NEXT: return %0
+  return %12 : $Bool
 }
 
 // CHECK-LABEL: sil @cross_block_load_promotion