DI: New analysis to replace the 'self consumed' analysis

slavapestov · slavapestov · commit 18c29b0cb430 · 2017-10-20T16:10:32.000-07:00
In a throwing or failable initializer for a class, the typical pattern
is that an apply or try_apply consumes the self value, and returns
success or failure. On success, a new self value is produced.
On failure, there is no new self value. In both cases, the original
self value no longer exists.

We used to model this by attempting to look at the apply or try_apply
instruction, and figure out from subsequent control flow which
successor block was the success case and which was the error case.

The error blocks were marked as such, and a dataflow analysis was used
to compute whether 'self' had been consumed in each block reachable
from the entry block.

This analysis was used to prevent invalid use of 'self' in catch
blocks when the initializer delegation was wrapped in do/catch;
more importantly, it was also used to know when to release 'self'
on exit from the initializer.

For example, when we 'throw e' here, 'self' was already consumed
and does not need to be released -- doing so would cause a crash:

do {
  try self.init(...)
} catch let e {
  // do some other cleanup
  throw e
}

On the other hand, here we do have to release 'self', otherwise we
will exit leaking memory:

do {
  try someOtherThing()
  self.init(...)
} catch let e {
  // do some other cleanup
  throw e
}

The problem with the old analysis is that it was too brittle and did
not recognize certain patterns generated by SILGen. For example, it
did not correctly detect the failure block of a delegation to a
foreign throwing initializer, because those are not modeled as a
try_apply; instead, they return an Optional value.

For similar reasons, we did not correctly failure blocks emitted
after calls to initializers which are both throwing and failable.

The new analysis is simpler and more robust. The idea is that in the
success block, SILGen emits a store of the new 'self' value into
the self box. So all we need to do is seed the dataflow analysis with
the set of blocks where the 'self' box is stored to, excluding the
initial entry block.

The new analysis is called 'self initialized' rather than 'self
consumed'. In blocks dominated by the self.init() delegation,
the result is the logical not of the old analysis:

- If the old analysis said self was consumed, the new one says self
 is not initialized.

- If the old analysis said self was not consumed, the new analysis
  says that self *is* initialized.

- If the old analysis returned a partial result, the new analysis
  will also; it means the block in question can be reached from
  blocks where the 'self' box is both initialized and not.

Note that any blocks that precede the self.init() delegation now
report self as uninitialized, because they are not dominated by
a store into the box. So any clients of the old analysis must first
check if self is "live", meaning we're past the point of the
self.init() call. Only if self is live do we then go on to check
the 'self initialized' analysis.
diff --git a/lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.cpp b/lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.cpp
@@ -426,6 +426,10 @@ bool DIMemoryUse::onlyTouchesTrivialElements(
 //                      DIElementUseInfo Implementation
 //===----------------------------------------------------------------------===//
 
+void DIElementUseInfo::trackStoreToSelf(SILInstruction *I) {
+  StoresToSelf.push_back(I);
+}
+
 void DIElementUseInfo::trackFailableInitCall(
     const DIMemoryObjectInfo &MemoryInfo, SILInstruction *I) {
   // If we have a store to self inside the normal BB, we have a 'real'
@@ -1106,7 +1110,7 @@ void ElementUseCollector::collectClassSelfUses() {
   // Okay, given that we have a proper setup, we walk the use chains of the self
   // box to find any accesses to it. The possible uses are one of:
   //
-  //   1) The initialization store (TheStore).
+  //   1) The initialization store.
   //   2) Loads of the box, which have uses of self hanging off of them.
   //   3) An assign to the box, which happens at super.init.
   //   4) Potential escapes after super.init, if self is closed over.
@@ -1115,10 +1119,26 @@ void ElementUseCollector::collectClassSelfUses() {
   for (auto *Op : MUI->getUses()) {
     SILInstruction *User = Op->getUser();
 
-    // Stores to self are initializations store or the rebind of self as
-    // part of the super.init call.  Ignore both of these.
-    if (isa<StoreInst>(User) && Op->getOperandNumber() == 1)
-      continue;
+    // Stores to self.
+    if (auto *SI = dyn_cast<StoreInst>(User)) {
+      if (Op->getOperandNumber() == 1) {
+        // The initial store of 'self' into the box at the start of the
+        // function. Ignore it.
+        if (auto *Arg = dyn_cast<SILArgument>(SI->getSrc()))
+          if (Arg->getParent() == MUI->getParent())
+            continue;
+
+        // A store of a load from the box is ignored.
+        // FIXME: SILGen should not emit these.
+        if (auto *LI = dyn_cast<LoadInst>(SI->getSrc()))
+          if (LI->getOperand() == MUI)
+            continue;
+
+        // Any other store needs to be recorded.
+        UseInfo.trackStoreToSelf(SI);
+        continue;
+      }
+    }
 
     // Ignore end_borrows. These can only come from us being the source of a
     // load_borrow.
@@ -1475,10 +1495,26 @@ void DelegatingInitElementUseCollector::collectClassInitSelfUses() {
     if (isa<EndBorrowInst>(User))
       continue;
 
-    // Stores to self are initializations store or the rebind of self as
-    // part of the super.init call.  Ignore both of these.
-    if (isa<StoreInst>(User) && Op->getOperandNumber() == 1)
-      continue;
+    // Stores to self.
+    if (auto *SI = dyn_cast<StoreInst>(User)) {
+      if (Op->getOperandNumber() == 1) {
+        // The initial store of 'self' into the box at the start of the
+        // function. Ignore it.
+        if (auto *Arg = dyn_cast<SILArgument>(SI->getSrc()))
+          if (Arg->getParent() == MUI->getParent())
+            continue;
+
+        // A store of a load from the box is ignored.
+        // FIXME: SILGen should not emit these.
+        if (auto *LI = dyn_cast<LoadInst>(SI->getSrc()))
+          if (LI->getOperand() == MUI)
+            continue;
+
+        // Any other store needs to be recorded.
+        UseInfo.trackStoreToSelf(SI);
+        continue;
+      }
+    }
 
     // For class initializers, the assign into the self box may be
     // captured as SelfInit or SuperInit elsewhere.
@@ -1491,6 +1527,7 @@ void DelegatingInitElementUseCollector::collectClassInitSelfUses() {
         if (auto *Fn = AI->getCalleeFunction()) {
           if (Fn->getRepresentation() ==
               SILFunctionTypeRepresentation::CFunctionPointer) {
+            UseInfo.trackStoreToSelf(User);
             UseInfo.trackUse(DIMemoryUse(User, DIUseKind::SelfInit, 0, 1));
             continue;
           }
@@ -1503,13 +1540,15 @@ void DelegatingInitElementUseCollector::collectClassInitSelfUses() {
     if (auto *AI = dyn_cast<AssignInst>(User)) {
       if (auto *AssignSource = AI->getOperand(0)->getDefiningInstruction()) {
         if (isSelfInitUse(AssignSource) || isSuperInitUse(AssignSource)) {
+          UseInfo.trackStoreToSelf(User);
           UseInfo.trackUse(DIMemoryUse(User, DIUseKind::SelfInit, 0, 1));
           continue;
         }
       }
       if (auto *AssignSource = dyn_cast<SILArgument>(AI->getOperand(0))) {
         if (AssignSource->getParent() == AI->getParent() &&
             (isSelfInitUse(AssignSource) || isSuperInitUse(AssignSource))) {
+          UseInfo.trackStoreToSelf(User);
           UseInfo.trackUse(DIMemoryUse(User, DIUseKind::SelfInit, 0, 1));
           continue;
         }
@@ -1573,13 +1612,17 @@ void DelegatingInitElementUseCollector::collectValueTypeInitSelfUses(
     // Stores *to* the allocation are writes.  If the value being stored is a
     // call to self.init()... then we have a self.init call.
     if (auto *AI = dyn_cast<AssignInst>(User)) {
-      if (AI->getDest() == I)
+      if (AI->getDest() == I) {
+        UseInfo.trackStoreToSelf(AI);
         Kind = DIUseKind::InitOrAssign;
+      }
     }
 
     if (auto *CAI = dyn_cast<CopyAddrInst>(User)) {
-      if (CAI->getDest() == I)
+      if (CAI->getDest() == I) {
+        UseInfo.trackStoreToSelf(CAI);
         Kind = DIUseKind::InitOrAssign;
+      }
     }
 
     // Look through begin_access
diff --git a/lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.h b/lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.h
@@ -150,6 +150,15 @@ class DIMemoryObjectInfo {
     return false;
   }
 
+  /// True if this memory object is the 'self' of a non-root class init method.
+  bool isNonRootClassSelf() const {
+    if (isClassInitSelf())
+      if (auto MUI = dyn_cast<MarkUninitializedInst>(MemoryInst))
+        return !MUI->isRootSelf();
+
+    return false;
+  }
+
   /// isDelegatingInit - True if this is a delegating initializer, one that
   /// calls 'self.init'.
   bool isDelegatingInit() const {
@@ -270,11 +279,14 @@ struct DIElementUseInfo {
   SmallVector<DIMemoryUse, 16> Uses;
   SmallVector<SILInstruction *, 4> Releases;
   TinyPtrVector<TermInst *> FailableInits;
+  TinyPtrVector<SILInstruction *> StoresToSelf;
 
   void trackUse(DIMemoryUse Use) { Uses.push_back(Use); }
 
   void trackDestroy(SILInstruction *Destroy) { Releases.push_back(Destroy); }
 
+  void trackStoreToSelf(SILInstruction *I);
+
   void trackFailableInitCall(const DIMemoryObjectInfo &TheMemory,
                              SILInstruction *I);
 
diff --git a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp
@@ -353,25 +353,33 @@ namespace {
 
     /// Helper flag used during building the worklist for the dataflow analysis.
     bool isInWorkList : 1;
-    
+
     /// Availability of elements within the block.
     /// Not "empty" for all blocks which have non-load uses or contain the
     /// definition of the memory object.
     AvailabilitySet LocalAvailability;
-    
+
     /// The live out information of the block. This is the LocalAvailability
     /// plus the information merged-in from the predecessor blocks.
     AvailabilitySet OutAvailability;
-    
+
     /// Keep track of blocks where the contents of the self box are not valid
     /// because we're in an error path dominated by a self.init or super.init
     /// delegation.
     Optional<DIKind> LocalSelfConsumed;
-    
+
     /// The live out information of the block. This is the LocalSelfConsumed
     /// plus the information merged-in from the predecessor blocks.
     Optional<DIKind> OutSelfConsumed;
 
+    /// Keep track of blocks where the contents of the self box are stored to
+    /// as a result of a successful self.init or super.init call.
+    Optional<DIKind> LocalSelfInitialized;
+
+    /// The live out information of the block. This is the LocalSelfInitialized
+    /// plus the information merged-in from the predecessor blocks.
+    Optional<DIKind> OutSelfInitialized;
+
     LiveOutBlockState(unsigned NumElements)
       : HasNonLoadUse(false),
         isInWorkList(false),
@@ -387,6 +395,10 @@ namespace {
         LocalSelfConsumed = DIKind::No;
       if (!OutSelfConsumed.hasValue())
         OutSelfConsumed = DIKind::No;
+      if (!LocalSelfInitialized.hasValue())
+        LocalSelfInitialized = DIKind::No;
+      if (!OutSelfInitialized.hasValue())
+        OutSelfInitialized = DIKind::No;
     }
 
     /// Transfer function for dataflow analysis.
@@ -429,13 +441,22 @@ namespace {
         }
       }
 
-      Optional<DIKind> result;
+      Optional<DIKind> temp;
       if (transferAvailability(Pred.OutSelfConsumed,
                                OutSelfConsumed,
                                LocalSelfConsumed,
+                               temp)) {
+        changed = true;
+        OutSelfConsumed = temp;
+      }
+
+      Optional<DIKind> result;
+      if (transferAvailability(Pred.OutSelfInitialized,
+                               OutSelfInitialized,
+                               LocalSelfInitialized,
                                result)) {
         changed = true;
-        OutSelfConsumed = result;
+        OutSelfInitialized = result;
       }
 
       return changed;
@@ -460,9 +481,17 @@ namespace {
       OutSelfConsumed = LocalSelfConsumed;
     }
 
+    /// Mark the block as storing to self, indicating the self box has been
+    /// initialized.
+    void markStoreToSelf() {
+      LocalSelfInitialized = DIKind::Yes;
+      OutSelfInitialized = DIKind::Yes;
+    }
+
     /// If true, we're not done with our dataflow analysis yet.
     bool containsUndefinedValues() {
       return (!OutSelfConsumed.hasValue() ||
+              !OutSelfInitialized.hasValue() ||
               OutAvailability.containsUnknownElements());
     }
   };
@@ -487,6 +516,7 @@ namespace {
 
     SmallVectorImpl<DIMemoryUse> &Uses;
     TinyPtrVector<TermInst *> &FailableInits;
+    TinyPtrVector<SILInstruction *> &StoresToSelf;
     SmallVectorImpl<SILInstruction *> &Destroys;
     std::vector<ConditionalDestroy> ConditionalDestroys;
 
@@ -537,6 +567,7 @@ namespace {
                                         unsigned NumElts);
 
     DIKind getSelfConsumedAtInst(SILInstruction *Inst);
+    DIKind getSelfInitializedAtInst(SILInstruction *Inst);
 
     bool isInitializedAtUse(const DIMemoryUse &Use,
                             bool *SuperInitDone = nullptr,
@@ -571,6 +602,7 @@ namespace {
     void computePredsLiveOut(SILBasicBlock *BB);
     void getOutAvailability(SILBasicBlock *BB, AvailabilitySet &Result);
     void getOutSelfConsumed(SILBasicBlock *BB, Optional<DIKind> &Result);
+    void getOutSelfInitialized(SILBasicBlock *BB, Optional<DIKind> &Result);
 
     bool shouldEmitError(SILInstruction *Inst);
     std::string getUninitElementName(const DIMemoryUse &Use);
@@ -589,7 +621,7 @@ LifetimeChecker::LifetimeChecker(const DIMemoryObjectInfo &TheMemory,
                                  DIElementUseInfo &UseInfo)
     : Module(TheMemory.MemoryInst->getModule()), TheMemory(TheMemory),
       Uses(UseInfo.Uses), FailableInits(UseInfo.FailableInits),
-      Destroys(UseInfo.Releases) {
+      StoresToSelf(UseInfo.StoresToSelf), Destroys(UseInfo.Releases) {
 
   // The first step of processing an element is to collect information about the
   // element into data structures we use later.
@@ -614,6 +646,13 @@ LifetimeChecker::LifetimeChecker(const DIMemoryObjectInfo &TheMemory,
     BBInfo.markAvailable(Use);
   }
 
+  // Mark blocks where the self box is initialized.
+  for (auto *I : StoresToSelf) {
+    // FIXME: critical edges?
+    auto *bb = I->getParent();
+    getBlockInfo(bb).markStoreToSelf();
+  }
+
   // Mark blocks where the self value has been consumed.
   for (auto *I : FailableInits) {
     auto *bb = I->getSuccessors()[1].getBB();
@@ -2460,6 +2499,14 @@ getOutSelfConsumed(SILBasicBlock *BB, Optional<DIKind> &Result) {
     Result = mergeKinds(Result, getBlockInfo(Pred).OutSelfConsumed);
 }
 
+void LifetimeChecker::
+getOutSelfInitialized(SILBasicBlock *BB, Optional<DIKind> &Result) {
+  computePredsLiveOut(BB);
+
+  for (auto *Pred : BB->getPredecessorBlocks())
+    Result = mergeKinds(Result, getBlockInfo(Pred).OutSelfInitialized);
+}
+
 AvailabilitySet
 LifetimeChecker::getLivenessAtNonTupleInst(swift::SILInstruction *Inst,
                                            swift::SILBasicBlock *InstBB,
@@ -2602,14 +2649,13 @@ DIKind LifetimeChecker::
 getSelfConsumedAtInst(SILInstruction *Inst) {
   DEBUG(llvm::dbgs() << "Get self consumed at " << *Inst);
 
-  Optional<DIKind> Result;
-
   SILBasicBlock *InstBB = Inst->getParent();
   auto &BlockInfo = getBlockInfo(InstBB);
 
   if (BlockInfo.LocalSelfConsumed.hasValue())
     return *BlockInfo.LocalSelfConsumed;
 
+  Optional<DIKind> Result;
   getOutSelfConsumed(InstBB, Result);
 
   // If the result wasn't computed, we must be analyzing code within
@@ -2621,6 +2667,45 @@ getSelfConsumedAtInst(SILInstruction *Inst) {
   return *Result;
 }
 
+/// getSelfInitializedAtInst - Check if the self box in an initializer has
+/// a fully initialized value at the specified instruction.
+///
+/// Possible outcomes:
+/// - 'Yes' -- 'self' is fully initialized, and should be destroyed in the
+///   usual manner in an error path
+///
+/// - 'No', and instruction is dominated by a SelfInit use -- this means
+///   'self' was consumed by a self.init or super.init call, and we're in
+///   an error path; there's nothing to clean up
+///
+/// - 'No', and instruction is not dominated by a SelfInit use -- this means
+///   we have to do a partial cleanup, for example deallocating a class
+///   instance without destroying its members
+///
+/// Also, the full range of conditional outcomes is possible above, if the
+/// result is 'Partial'.
+DIKind LifetimeChecker::
+getSelfInitializedAtInst(SILInstruction *Inst) {
+  DEBUG(llvm::dbgs() << "Get self initialized at " << *Inst);
+
+  SILBasicBlock *InstBB = Inst->getParent();
+  auto &BlockInfo = getBlockInfo(InstBB);
+
+  if (BlockInfo.LocalSelfInitialized.hasValue())
+    return *BlockInfo.LocalSelfInitialized;
+
+  Optional<DIKind> Result;
+  getOutSelfInitialized(InstBB, Result);
+
+  // If the result wasn't computed, we must be analyzing code within
+  // an unreachable cycle that is not dominated by "TheMemory".  Just force
+  // the result to initialized so that clients don't have to handle this.
+  if (!Result.hasValue())
+    Result = DIKind::Yes;
+
+  return *Result;
+}
+
 /// The specified instruction is a use of some number of elements.  Determine
 /// whether all of the elements touched by the instruction are definitely
 /// initialized at this point or not.
