Merge pull request #59591 from eeckstein/fix-dead-obj-elim

DeadObjectElimination: fix a bug which caused wrong inserted release instructions.

Author: eeckstein

lib/SILOptimizer/Transforms/DeadObjectElimination.cpp

@@ -368,16 +368,6 @@ static bool onlyStoresToTailObjects(BuiltinInst *destroyArray,
   return true;
 }
 
-/// Inserts releases of all stores in \p users.
-static void insertCompensatingReleases(SILInstruction *before,
-                                       const UserList &users) {
-  for (SILInstruction *user : users) {
-    if (auto *store = dyn_cast<StoreInst>(user)) {
-      createDecrementBefore(store->getSrc(), before);
-    }
-  }
-}
-
 /// Analyze the use graph of AllocRef for any uses that would prevent us from
 /// zapping it completely.
 static bool
@@ -736,6 +726,7 @@ class DeadObjectElimination : public SILFunctionTransform {
   llvm::DenseMap<SILType, DestructorEffects> DestructorAnalysisCache;
 
   InstructionDeleter deleter;
+  DominanceInfo *domInfo = nullptr;
 
   void removeInstructions(ArrayRef<SILInstruction*> toRemove);
 
@@ -745,6 +736,9 @@ class DeadObjectElimination : public SILFunctionTransform {
   bool processAllocBox(AllocBoxInst *ABI){ return false;}
   bool processAllocApply(ApplyInst *AI, DeadEndBlocks &DEBlocks);
 
+  bool insertCompensatingReleases(SILInstruction *before,
+                                  const UserList &users);
+
   bool getDeadInstsAfterInitializerRemoved(
     ApplyInst *AI, llvm::SmallVectorImpl<SILInstruction *> &ToDestroy);
   bool removeAndReleaseArray(
@@ -780,9 +774,12 @@ class DeadObjectElimination : public SILFunctionTransform {
     if (getFunction()->hasOwnership())
       return;
 
+    assert(!domInfo);
+
     if (processFunction(*getFunction())) {
       invalidateAnalysis(SILAnalysis::InvalidationKind::CallsAndInstructions);
     }
+    domInfo = nullptr;
   }
 
 };
@@ -841,8 +838,11 @@ bool DeadObjectElimination::processAllocRef(AllocRefInstBase *ARI) {
       releaseOfTailElems = user;
     }
   }
-  if (releaseOfTailElems)
-    insertCompensatingReleases(releaseOfTailElems, UsersToRemove);
+  if (releaseOfTailElems) {
+    if (!insertCompensatingReleases(releaseOfTailElems, UsersToRemove)) {
+      return false;
+    }
+  }
 
   // Remove the AllocRef and all of its users.
   removeInstructions(
@@ -1052,6 +1052,33 @@ bool DeadObjectElimination::processAllocApply(ApplyInst *AI,
   return true;
 }
 
+/// Inserts releases of all stores in \p users.
+/// Returns false, if this is not possible.
+bool DeadObjectElimination::insertCompensatingReleases(SILInstruction *before,
+                                                       const UserList &users) {
+
+  // First check if all stored values dominate the release-point.
+  for (SILInstruction *user : users) {
+    if (auto *store = dyn_cast<StoreInst>(user)) {
+      if (!domInfo) {
+        domInfo = getAnalysis<DominanceAnalysis>()->get(before->getFunction());
+      }
+      SILBasicBlock *srcBlock = store->getSrc()->getParentBlock();
+      if (!domInfo->dominates(srcBlock, before->getParent()))
+        return false;
+    }
+  }
+
+  // Second, create the releases.
+  for (SILInstruction *user : users) {
+    if (auto *store = dyn_cast<StoreInst>(user)) {
+      createDecrementBefore(store->getSrc(), before);
+    }
+  }
+  return true;
+}
+
+
 //===----------------------------------------------------------------------===//
 //                              Top Level Driver
 //===----------------------------------------------------------------------===//

test/SILOptimizer/dead_alloc_elim.sil

@@ -669,3 +669,49 @@ bb0:
   return %r : $()
 }
 
+sil @createit : $@convention(thin) () -> @owned Kl
+
+// Check that we don't crash here.
+sil @not_dominating_stores : $@convention(thin) () -> () {
+bb0:
+  %7 = integer_literal $Builtin.Word, 1
+  %10 = alloc_ref [stack] [tail_elems $Kl * %7 : $Builtin.Word] $_ContiguousArrayStorage<Kl>
+  %11 = upcast %10 : $_ContiguousArrayStorage<Kl> to $__ContiguousArrayStorageBase
+  %18 = ref_tail_addr %11 : $__ContiguousArrayStorageBase, $Kl
+  %35 = function_ref @createit : $@convention(thin) () -> @owned Kl
+  cond_br undef, bb1, bb2
+bb1:
+  %36 = apply %35() : $@convention(thin) () -> @owned Kl
+  store %36 to %18 : $*Kl
+  br bb3
+bb2:
+  %46 = apply %35() : $@convention(thin) () -> @owned Kl
+  store %46 to %18 : $*Kl
+  br bb3
+bb3:
+  strong_release %10 : $_ContiguousArrayStorage<Kl>
+  dealloc_stack_ref %10 : $_ContiguousArrayStorage<Kl>
+  %1 = tuple()
+  return %1 : $()
+}
+
+sil @$ss23_ContiguousArrayStorageCfD : $@convention(method) <Element> (@owned _ContiguousArrayStorage<Element>) -> () {
+bb0(%0 : $_ContiguousArrayStorage<Element>):
+  %1 = ref_tail_addr %0 : $_ContiguousArrayStorage<Element>, $Element
+  %2 = address_to_pointer %1 : $*Element to $Builtin.RawPointer
+  %3 = upcast %0 : $_ContiguousArrayStorage<Element> to $__ContiguousArrayStorageBase
+  %4 = ref_element_addr %3 : $__ContiguousArrayStorageBase, #__ContiguousArrayStorageBase.countAndCapacity
+  %5 = struct_element_addr %4 : $*_ArrayBody, #_ArrayBody._storage
+  %6 = struct_element_addr %5 : $*_SwiftArrayBodyStorage, #_SwiftArrayBodyStorage.count
+  %7 = struct_element_addr %6 : $*Int, #Int._value
+  %8 = load %7 : $*Builtin.Int64
+  %9 = builtin "assumeNonNegative_Int64"(%8 : $Builtin.Int64) : $Builtin.Int64
+  %10 = metatype $@thick Element.Type
+  %11 = builtin "truncOrBitCast_Int64_Word"(%9 : $Builtin.Int64) : $Builtin.Word
+  %12 = builtin "destroyArray"<Element>(%10 : $@thick Element.Type, %2 : $Builtin.RawPointer, %11 : $Builtin.Word) : $()
+  fix_lifetime %0 : $_ContiguousArrayStorage<Element>
+  dealloc_ref %0 : $_ContiguousArrayStorage<Element>
+  %15 = tuple ()
+  return %15 : $()
+}
+