[silcombine] (store (struct_gep addr) value) -> (store addr (struct v…

lib/SILOptimizer/IPO/GlobalOpt.cpp

@@ -137,9 +137,6 @@ class SILGlobalOpt {
   /// can be statically initialized.
   void optimizeInitializer(SILFunction *AddrF, GlobalInitCalls &Calls);
 
-  /// Perform peephole optimizations on the initializer list.
-  void peepholeInitializer(SILFunction *InitFunc);
-
   /// Optimize access to the global variable, which is known to have a constant
   /// value. Replace all loads from the global address by invocations of a
   /// getter that returns the value of this variable.
@@ -735,8 +732,6 @@ void SILGlobalOpt::optimizeInitializer(SILFunction *AddrF,
       InitializerCount[InitF] > 1)
     return;
 
-  peepholeInitializer(InitF);
-
   // If the globalinit_func is trivial, continue; otherwise bail.
   SingleValueInstruction *InitVal;
   SILGlobalVariable *SILG = getVariableOfStaticInitializer(InitF, InitVal);
@@ -759,58 +754,6 @@ void SILGlobalOpt::optimizeInitializer(SILFunction *AddrF,
   HasChanged = true;
 }
 
-void SILGlobalOpt::peepholeInitializer(SILFunction *InitFunc) {
-  if (InitFunc->size() != 1)
-    return;
-  SILBasicBlock *BB = &InitFunc->front();
-
-  for (auto &I : *BB) {
-    if (auto *SI = dyn_cast<StoreInst>(&I)) {
-
-      // If struct S has a single field, replace
-      //   %a = struct_element_addr %s : $*S
-      //   store %x to %a
-      // with
-      //   %y = struct $S (%x)
-      //   store %y to %s
-      //
-      // This pattern occurs with resilient static properties, like
-      // struct ResilientStruct {
-      //   var singleField: Int
-      //   public static let x = ResilientStruct(singleField: 27)
-      // }
-      //
-      // TODO: handle structs with multiple fields.
-      SILValue Addr = SI->getDest();
-      auto *SEA = dyn_cast<StructElementAddrInst>(Addr);
-      if (!SEA)
-        continue;
-
-      if (SEA->getOperand()->getType().isAddressOnly(InitFunc))
-        continue;
-
-      StructDecl *Decl = SEA->getStructDecl();
-      auto beginProp = Decl->getStoredProperties().begin();
-      if (std::next(beginProp) != Decl->getStoredProperties().end())
-        continue;
-
-      assert(*beginProp == SEA->getField());
-
-      SILBuilder B(SI);
-      SILValue StructAddr = SEA->getOperand();
-      StructInst *Struct = B.createStruct(SEA->getLoc(),
-                                          StructAddr->getType().getObjectType(),
-                                          { SI->getSrc() });
-      SI->setOperand(StoreInst::Src, Struct);
-      SI->setOperand(StoreInst::Dest, StructAddr);
-      if (SEA->use_empty()) {
-        SEA->eraseFromParent();
-      }
-      HasChanged = true;
-    }
-  }
-}
-
 static bool canBeChangedExternally(SILGlobalVariable *SILG) {
   // Don't assume anything about globals which are imported from other modules.
   if (isAvailableExternally(SILG->getLinkage()))

lib/SILOptimizer/SILCombiner/SILCombiner.h

@@ -220,6 +220,7 @@ class SILCombiner :
   SILInstruction *visitUpcastInst(UpcastInst *UCI);
   SILInstruction *optimizeLoadFromStringLiteral(LoadInst *LI);
   SILInstruction *visitLoadInst(LoadInst *LI);
+  SILInstruction *visitStoreInst(StoreInst *si);
   SILInstruction *visitIndexAddrInst(IndexAddrInst *IA);
   SILInstruction *visitAllocStackInst(AllocStackInst *AS);
   SILInstruction *visitAllocRefInst(AllocRefInst *AR);

lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp

@@ -15,6 +15,7 @@
 #include "swift/Basic/STLExtras.h"
 #include "swift/SIL/DebugUtils.h"
 #include "swift/SIL/DynamicCasts.h"
+#include "swift/SIL/InstructionUtils.h"
 #include "swift/SIL/PatternMatch.h"
 #include "swift/SIL/Projection.h"
 #include "swift/SIL/SILBuilder.h"
@@ -585,6 +586,79 @@ SILInstruction *SILCombiner::optimizeLoadFromStringLiteral(LoadInst *LI) {
   return Builder.createIntegerLiteral(LI->getLoc(), LI->getType(), str[index]);
 }
 
+SILInstruction *SILCombiner::visitStoreInst(StoreInst *si) {
+  auto *f = si->getFunction();
+  assert(f->getConventions().useLoweredAddresses() &&
+         "These optimizations assume that opaque values are not enabled");
+
+  // (store (struct_element_addr addr) object)
+  //   ->
+  // (store addr (struct object))
+
+  // If our store's destination is not a struct_element_addr, bail early.
+  auto *sea = dyn_cast<StructElementAddrInst>(si->getDest());
+  if (!sea)
+    return nullptr;
+
+  // Ok, we have at least one struct_element_addr. Canonicalize the underlying
+  // store.
+  Builder.setInsertionPoint(si);
+  SILLocation loc = si->getLoc();
+
+  auto &mod = si->getModule();
+  SILValue result = si->getSrc();
+  SILValue iterAddr = sea->getOperand();
+  SILValue storeAddr;
+  while (true) {
+    SILType iterAddrType = iterAddr->getType();
+
+    // If our aggregate has unreferenced storage then we can never prove if it
+    // actually has a single field.
+    if (iterAddrType.aggregateHasUnreferenceableStorage())
+      break;
+
+    auto *decl = iterAddrType.getStructOrBoundGenericStruct();
+    assert(
+        !decl->isResilient(mod.getSwiftModule(), f->getResilienceExpansion()) &&
+        "This code assumes resilient structs can not have fragile fields. If "
+        "this assert is hit, this has been changed. Please update this code.");
+
+    // NOTE: If this is ever changed to support enums, we must check for address
+    // only types here. For structs we do not have to check since a single
+    // element struct with a loadable element can never be address only. We
+    // additionally do not have to worry about our input value being address
+    // only since we are storing into it.
+    auto props = decl->getStoredProperties();
+    if (std::next(props.begin()) != props.end())
+      break;
+
+    // Update the store location now that we know it is safe.
+    storeAddr = iterAddr;
+
+    // Otherwise, create the struct.
+    result = Builder.createStruct(loc, iterAddrType.getObjectType(), result);
+
+    // See if we have another struct_element_addr we can strip off. If we don't
+    // then this as much as we can promote.
+    sea = dyn_cast<StructElementAddrInst>(sea->getOperand());
+    if (!sea)
+      break;
+    iterAddr = sea->getOperand();
+  }
+
+  // If we failed to create any structs, bail.
+  if (result == si->getSrc())
+    return nullptr;
+
+  // Then create a new store, storing the value into the relevant computed
+  // address.
+  Builder.createStore(loc, result, storeAddr,
+                      StoreOwnershipQualifier::Unqualified);
+
+  // Then eliminate the original store.
+  return eraseInstFromFunction(*si);
+}
+
 SILInstruction *SILCombiner::visitLoadInst(LoadInst *LI) {
   // (load (upcast-ptr %x)) -> (upcast-ref (load %x))
   Builder.setCurrentDebugScope(LI->getDebugScope());

test/SILOptimizer/sil_combine.sil

@@ -285,8 +285,8 @@ bb3(%3 : $BoolLike):
 // CHECK-NEXT:  [[IN_GEP:%[0-9]+]] = struct_element_addr [[IN]] : $*UInt8, #UInt8._value
 // CHECK-NEXT:  [[IN_LOADED:%[0-9]+]] = load [[IN_GEP]] : $*Builtin.Int8
 // CHECK-NEXT:  [[LITERAL:%[0-9]+]] = integer_literal $Builtin.Int8, 1
-// CHECK-NEXT:  [[IN_GEP_STORE:%[0-9]+]] = struct_element_addr [[IN]] : $*UInt8, #UInt8._value
-// CHECK-NEXT:  store [[LITERAL]] to [[IN_GEP_STORE]] : $*Builtin.Int8
+// CHECK-NEXT:  [[UINT8:%.*]] = struct $UInt8 ([[LITERAL]] : $Builtin.Int8)
+// CHECK-NEXT:  store [[UINT8]] to [[IN]] : $*UInt8
 // CHECK-NEXT:  return [[IN_LOADED]] : $Builtin.Int8
 sil @struct_extract_load_to_load_struct_element_addr : $@convention(thin) (@inout UInt8) -> (Builtin.Int8) {
 bb0(%0 : $*UInt8):
@@ -322,7 +322,7 @@ bb0(%0 : $*(Builtin.Int8, Builtin.Int8)):
 // CHECK: bb0([[IN:%[0-9]+]] : $*UInt8):
 // CHECK-NEXT: load
 // CHECK-NEXT: integer_literal
-// CHECK-NEXT: struct_element_addr
+// CHECK-NEXT: struct
 // CHECK-NEXT: store
 // CHECK-NEXT: struct_extract
 // CHECK-NEXT: tuple

test/SILOptimizer/sil_combine_memopts.sil

@@ -0,0 +1,122 @@
+// RUN: %target-sil-opt -enable-sil-verify-all -sil-combine %s | %FileCheck %s
+
+// This file tests sil combine's canonicalization of memory.
+
+class Klass {}
+
+////////////////////////////
+// Store Canonicalization //
+////////////////////////////
+
+// We canonicalize stores of fields of single element structs into stores of the
+// struct itself. The two are equivalent.
+
+struct SingleEltStruct {
+  var k: Klass
+}
+
+// CHECK-LABEL: sil @promote_initialization_of_single_elt_struct : $@convention(thin) (@owned Klass) -> () {
+// CHECK: bb0([[ARG:%.*]] : $Klass):
+// CHECK:   [[STACK:%.*]] = alloc_stack $SingleEltStruct
+// CHECK:   [[STRUCT_ARG:%.*]] = struct $SingleEltStruct ([[ARG]] : $Klass)
+// CHECK:   store [[STRUCT_ARG]] to [[STACK]]
+// CHECK:   dealloc_stack [[STACK]]
+// CHECK: } // end sil function 'promote_initialization_of_single_elt_struct'
+sil @promote_initialization_of_single_elt_struct : $@convention(thin) (@owned Klass) -> () {
+bb0(%0 : $Klass):
+  %1 = alloc_stack $SingleEltStruct
+  %2 = struct_element_addr %1 : $*SingleEltStruct, #SingleEltStruct.k
+  store %0 to %2 : $*Klass
+  dealloc_stack %1 : $*SingleEltStruct
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+struct RecursiveSingleEltStruct {
+  var field: RecursiveSingleEltStructField
+}
+
+struct RecursiveSingleEltStructField {
+  var k: Klass
+}
+
+// CHECK-LABEL: sil @promote_initialization_of_recursive_single_elt_struct : $@convention(thin) (@owned Klass) -> () {
+// CHECK: bb0([[ARG:%.*]] : $Klass):
+// CHECK:   [[STACK:%.*]] = alloc_stack $RecursiveSingleEltStruct
+// CHECK:   [[STRUCT_ARG_1:%.*]] = struct $RecursiveSingleEltStructField ([[ARG]] : $Klass)
+// CHECK:   [[STRUCT_ARG_2:%.*]] = struct $RecursiveSingleEltStruct ([[STRUCT_ARG_1]] : $RecursiveSingleEltStructField)
+// CHECK:   store [[STRUCT_ARG_2]] to [[STACK]]
+// CHECK:   dealloc_stack [[STACK]]
+// CHECK: } // end sil function 'promote_initialization_of_recursive_single_elt_struct'
+sil @promote_initialization_of_recursive_single_elt_struct : $@convention(thin) (@owned Klass) -> () {
+bb0(%0 : $Klass):
+  %1 = alloc_stack $RecursiveSingleEltStruct
+  %2 = struct_element_addr %1 : $*RecursiveSingleEltStruct, #RecursiveSingleEltStruct.field
+  %3 = struct_element_addr %2 : $*RecursiveSingleEltStructField, #RecursiveSingleEltStructField.k
+  store %0 to %3 : $*Klass
+  %4 = load %1 : $*RecursiveSingleEltStruct
+  release_value %4 : $RecursiveSingleEltStruct
+  dealloc_stack %1 : $*RecursiveSingleEltStruct
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+struct MultipleFieldStruct {
+  var k: Klass
+  var field: RecursiveSingleEltStructField
+}
+
+// CHECK-LABEL: sil @only_promote_as_far_as_have_single_elts : $@convention(thin) (@owned Klass) -> () {
+// CHECK: bb0([[ARG:%.*]] :
+// CHECK:   [[STACK:%.*]] = alloc_stack $MultipleFieldStruct
+// CHECK:   [[MULTIPLE_FIELD_SEA:%.*]] = struct_element_addr [[STACK]]
+// CHECK:   [[VALUE:%.*]] = struct $RecursiveSingleEltStructField ([[ARG]] : $Klass)
+// CHECK:   store [[VALUE]] to [[MULTIPLE_FIELD_SEA]]
+// CHECK:   dealloc_stack [[STACK]]
+// CHECK: } // end sil function 'only_promote_as_far_as_have_single_elts'
+sil @only_promote_as_far_as_have_single_elts : $@convention(thin) (@owned Klass) -> () {
+bb0(%0 : $Klass):
+  %1 = alloc_stack $MultipleFieldStruct
+  %2 = struct_element_addr %1 : $*MultipleFieldStruct, #MultipleFieldStruct.field
+  %3 = struct_element_addr %2 : $*RecursiveSingleEltStructField, #RecursiveSingleEltStructField.k
+  store %0 to %3 : $*Klass
+  dealloc_stack %1 : $*MultipleFieldStruct
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+// CHECK-LABEL: sil @perform_no_work_if_multiple_fields : $@convention(thin) (@owned RecursiveSingleEltStructField) -> () {
+// CHECK: bb0([[ARG:%.*]] :
+// CHECK:   [[STACK:%.*]] = alloc_stack $MultipleFieldStruct
+// CHECK:   [[MULTIPLE_FIELD_SEA:%.*]] = struct_element_addr [[STACK]]
+// CHECK:   store [[ARG]] to [[MULTIPLE_FIELD_SEA]]
+// CHECK:   dealloc_stack [[STACK]]
+// CHECK: } // end sil function 'perform_no_work_if_multiple_fields'
+sil @perform_no_work_if_multiple_fields : $@convention(thin) (@owned RecursiveSingleEltStructField) -> () {
+bb0(%0 : $RecursiveSingleEltStructField):
+  %1 = alloc_stack $MultipleFieldStruct
+  %2 = struct_element_addr %1 : $*MultipleFieldStruct, #MultipleFieldStruct.field
+  store %0 to %2 : $*RecursiveSingleEltStructField
+  dealloc_stack %1 : $*MultipleFieldStruct
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+// CHECK-LABEL: sil @only_promote_while_we_have_sea : $@convention(thin) (@owned Klass) -> () {
+// CHECK: bb0([[ARG:%.*]] :
+// CHECK:   [[STACK:%.*]] = alloc_stack $(Klass, RecursiveSingleEltStructField)
+// CHECK:   [[TUPLE_ADDR:%.*]] = tuple_element_addr [[STACK]]
+// CHECK:   [[VALUE:%.*]] = struct $RecursiveSingleEltStructField ([[ARG]] : $Klass)
+// CHECK:   store [[VALUE]] to [[TUPLE_ADDR]]
+// CHECK:   dealloc_stack [[STACK]]
+// CHECK: } // end sil function 'only_promote_while_we_have_sea'
+sil @only_promote_while_we_have_sea : $@convention(thin) (@owned Klass) -> () {
+bb0(%0 : $Klass):
+  %1 = alloc_stack $(Klass, RecursiveSingleEltStructField)
+  %2 = tuple_element_addr %1 : $*(Klass, RecursiveSingleEltStructField), 1
+  %3 = struct_element_addr %2 : $*RecursiveSingleEltStructField, #RecursiveSingleEltStructField.k
+  store %0 to %3 : $*Klass
+  dealloc_stack %1 : $*(Klass, RecursiveSingleEltStructField)
+  %9999 = tuple()
+  return %9999 : $()
+}