Merge pull request #17234 from gottesmm/pr-0e0ac4897be5b6059f2959956c708dea2e027b88

Author: swift-ci

include/swift/SILOptimizer/Analysis/ARCAnalysis.h

@@ -13,16 +13,17 @@
 #ifndef SWIFT_SILOPTIMIZER_ANALYSIS_ARCANALYSIS_H
 #define SWIFT_SILOPTIMIZER_ANALYSIS_ARCANALYSIS_H
 
+#include "swift/Basic/LLVM.h"
 #include "swift/SIL/SILArgument.h"
-#include "swift/SIL/SILValue.h"
 #include "swift/SIL/SILBasicBlock.h"
+#include "swift/SIL/SILValue.h"
 #include "swift/SILOptimizer/Analysis/AliasAnalysis.h"
 #include "swift/SILOptimizer/Analysis/PostOrderAnalysis.h"
 #include "swift/SILOptimizer/Analysis/RCIdentityAnalysis.h"
 #include "llvm/ADT/BitVector.h"
-#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/TinyPtrVector.h"
 
 namespace swift {
@@ -39,15 +40,13 @@ class SILFunction;
 } // end namespace swift
 
 namespace swift {
+
 /// Return true if this is a retain instruction.
 bool isRetainInstruction(SILInstruction *II);
 
 /// Return true if this is a release instruction.
 bool isReleaseInstruction(SILInstruction *II);
 
-using RetainList = TinyPtrVector<SILInstruction *>;
-using ReleaseList = TinyPtrVector<SILInstruction *>;
-
 /// \returns True if the user \p User decrements the ref count of \p Ptr.
 bool mayDecrementRefCount(SILInstruction *User, SILValue Ptr,
                           AliasAnalysis *AA);
@@ -141,16 +140,11 @@ class ConsumedResultToEpilogueRetainMatcher {
   SILFunction *F;
   RCIdentityFunctionInfo *RCFI;
   AliasAnalysis *AA;
+
   // We use a list of instructions for now so that we can keep the same interface
   // and handle exploded retain_value later.
-  RetainList EpilogueRetainInsts;
+  TinyPtrVector<SILInstruction *> EpilogueRetainInsts;
 
-  /// Return true if all the successors of the EpilogueRetainInsts do not have
-  /// a retain. 
-  bool isTransitiveSuccessorsRetainFree(llvm::DenseSet<SILBasicBlock *> BBs);
-
-  /// Finds matching releases in the provided block \p BB.
-  RetainKindValue findMatchingRetainsInBasicBlock(SILBasicBlock *BB, SILValue V);
 public:
   /// Finds matching releases in the return block of the function \p F.
   ConsumedResultToEpilogueRetainMatcher(RCIdentityFunctionInfo *RCFI,
@@ -160,7 +154,9 @@ class ConsumedResultToEpilogueRetainMatcher {
   /// Finds matching releases in the provided block \p BB.
   void findMatchingRetains(SILBasicBlock *BB);
 
-  RetainList getEpilogueRetains() { return EpilogueRetainInsts; }
+  ArrayRef<SILInstruction *> getEpilogueRetains() const {
+    return EpilogueRetainInsts;
+  }
 
   /// Recompute the mapping from argument to consumed arg.
   void recompute();
@@ -182,6 +178,16 @@ class ConsumedResultToEpilogueRetainMatcher {
   unsigned size() const { return EpilogueRetainInsts.size(); }
 
   iterator_range<iterator> getRange() { return swift::make_range(begin(), end()); }
+
+private:
+  /// Return true if all the successors of the EpilogueRetainInsts do not have
+  /// a retain.
+  bool
+  isTransitiveSuccessorsRetainFree(const llvm::DenseSet<SILBasicBlock *> &BBs);
+
+  /// Finds matching releases in the provided block \p BB.
+  RetainKindValue findMatchingRetainsInBasicBlock(SILBasicBlock *BB,
+                                                  SILValue V);
 };
 
 /// A class that attempts to match owned arguments and corresponding epilogue
@@ -197,40 +203,16 @@ class ConsumedArgToEpilogueReleaseMatcher {
   RCIdentityFunctionInfo *RCFI;
   ExitKind Kind;
   ArrayRef<SILArgumentConvention> ArgumentConventions;
-  llvm::SmallMapVector<SILArgument *, ReleaseList, 8> ArgInstMap;
+
+  using InstListTy = TinyPtrVector<SILInstruction *>;
+  llvm::SmallMapVector<SILArgument *, InstListTy, 8> ArgInstMap;
 
   /// Set to true if we found some releases but not all for the argument.
   llvm::DenseSet<SILArgument *> FoundSomeReleases;
 
   /// Eventually this will be used in place of HasBlock.
   SILBasicBlock *ProcessedBlock;
 
-  /// Return true if we have seen releases to part or all of \p Derived in
-  /// \p Insts.
-  /// 
-  /// NOTE: This function relies on projections to analyze the relation
-  /// between the releases values in \p Insts and \p Derived, it also bails
-  /// out and return true if projection path can not be formed between Base
-  /// and any one the released values.
-  bool isRedundantRelease(ReleaseList Insts, SILValue Base, SILValue Derived);
-
-  /// Return true if we have a release instruction for all the reference
-  /// semantics part of \p Argument.
-  bool releaseArgument(ReleaseList Insts, SILValue Argument);
-
-  /// Walk the basic block and find all the releases that match to function
-  /// arguments. 
-  void collectMatchingReleases(SILBasicBlock *BB);
-
-  /// Walk the function and find all the destroy_addr instructions that match
-  /// to function arguments. 
-  void collectMatchingDestroyAddresses(SILBasicBlock *BB);
-
-  /// For every argument in the function, check to see whether all epilogue
-  /// releases are found. Clear all releases for the argument if not all 
-  /// epilogue releases are found.
-  void processMatchingReleases();
-
 public:
   /// Finds matching releases in the return block of the function \p F.
   ConsumedArgToEpilogueReleaseMatcher(
@@ -288,33 +270,32 @@ class ConsumedArgToEpilogueReleaseMatcher {
     return getSingleReleaseForArgument(Arg);
   }
 
-  ReleaseList getReleasesForArgument(SILArgument *Arg) {
-    ReleaseList Releases;
+  ArrayRef<SILInstruction *> getReleasesForArgument(SILArgument *Arg) {
     auto I = ArgInstMap.find(Arg);
     if (I == ArgInstMap.end())
-      return Releases;
-    return I->second; 
+      return {};
+    return I->second;
   }
 
-  ReleaseList getReleasesForArgument(SILValue V) {
-    ReleaseList Releases;
+  ArrayRef<SILInstruction *> getReleasesForArgument(SILValue V) {
     auto *Arg = dyn_cast<SILArgument>(V);
     if (!Arg)
-      return Releases;
+      return {};
     return getReleasesForArgument(Arg);
   }
 
   /// Recompute the mapping from argument to consumed arg.
   void recompute();
 
   bool isSingleReleaseMatchedToArgument(SILInstruction *Inst) {
-    auto Pred = [&Inst](const std::pair<SILArgument *,
-                                        ReleaseList> &P) -> bool {
-      if (P.second.size() > 1)
-        return false;
-      return *P.second.begin() == Inst;
-    };
-    return count_if(ArgInstMap, Pred);
+    return count_if(
+        ArgInstMap,
+        [&Inst](const std::pair<SILArgument *, ArrayRef<SILInstruction *>> &P)
+            -> bool {
+          if (P.second.size() > 1)
+            return false;
+          return *P.second.begin() == Inst;
+        });
   }
 
   using iterator = decltype(ArgInstMap)::iterator;
@@ -334,6 +315,34 @@ class ConsumedArgToEpilogueReleaseMatcher {
   unsigned size() const { return ArgInstMap.size(); }
 
   iterator_range<iterator> getRange() { return swift::make_range(begin(), end()); }
+
+private:
+  /// Return true if we have seen releases to part or all of \p Derived in
+  /// \p Insts.
+  ///
+  /// NOTE: This function relies on projections to analyze the relation
+  /// between the releases values in \p Insts and \p Derived, it also bails
+  /// out and return true if projection path can not be formed between Base
+  /// and any one the released values.
+  bool isRedundantRelease(ArrayRef<SILInstruction *> Insts, SILValue Base,
+                          SILValue Derived);
+
+  /// Return true if we have a release instruction for all the reference
+  /// semantics part of \p Argument.
+  bool releaseArgument(ArrayRef<SILInstruction *> Insts, SILValue Argument);
+
+  /// Walk the basic block and find all the releases that match to function
+  /// arguments.
+  void collectMatchingReleases(SILBasicBlock *BB);
+
+  /// Walk the function and find all the destroy_addr instructions that match
+  /// to function arguments.
+  void collectMatchingDestroyAddresses(SILBasicBlock *BB);
+
+  /// For every argument in the function, check to see whether all epilogue
+  /// releases are found. Clear all releases for the argument if not all
+  /// epilogue releases are found.
+  void processMatchingReleases();
 };
 
 class ReleaseTracker {

lib/SILOptimizer/Analysis/ARCAnalysis.cpp

@@ -498,22 +498,24 @@ void ConsumedResultToEpilogueRetainMatcher::recompute() {
   findMatchingRetains(&*BB);
 }
 
-bool
-ConsumedResultToEpilogueRetainMatcher::
-isTransitiveSuccessorsRetainFree(llvm::DenseSet<SILBasicBlock *> BBs) {
+bool ConsumedResultToEpilogueRetainMatcher::isTransitiveSuccessorsRetainFree(
+    const llvm::DenseSet<SILBasicBlock *> &BBs) {
   // For every block with retain, we need to check the transitive
   // closure of its successors are retain-free.
   for (auto &I : EpilogueRetainInsts) {
-    auto *CBB = I->getParent();
-    for (auto &Succ : CBB->getSuccessors()) {
-      if (BBs.find(Succ) != BBs.end())
+    for (auto &Succ : I->getParent()->getSuccessors()) {
+      if (BBs.count(Succ))
         continue;
       return false;
     }
   }
+
+  // FIXME: We are iterating over a DenseSet. That can lead to non-determinism
+  // and is in general pretty inefficient since we are iterating over a hash
+  // table.
   for (auto CBB : BBs) {
     for (auto &Succ : CBB->getSuccessors()) {
-      if (BBs.find(Succ) != BBs.end())
+      if (BBs.count(Succ))
         continue;
       return false;
     }
@@ -707,9 +709,8 @@ void ConsumedArgToEpilogueReleaseMatcher::recompute() {
   findMatchingReleases(&*BB);
 }
 
-bool
-ConsumedArgToEpilogueReleaseMatcher::
-isRedundantRelease(ReleaseList Insts, SILValue Base, SILValue Derived) {
+bool ConsumedArgToEpilogueReleaseMatcher::isRedundantRelease(
+    ArrayRef<SILInstruction *> Insts, SILValue Base, SILValue Derived) {
   // We use projection path to analyze the relation.
   auto POp = ProjectionPath::getProjectionPath(Base, Derived);
   // We can not build a projection path from the base to the derived, bail out.
@@ -730,9 +731,8 @@ isRedundantRelease(ReleaseList Insts, SILValue Base, SILValue Derived) {
   return false;
 }
 
-bool
-ConsumedArgToEpilogueReleaseMatcher::
-releaseArgument(ReleaseList Insts, SILValue Arg) {
+bool ConsumedArgToEpilogueReleaseMatcher::releaseArgument(
+    ArrayRef<SILInstruction *> Insts, SILValue Arg) {
   // Reason about whether all parts are released.
   SILModule *Mod = &(*Insts.begin())->getModule();
 

lib/SILOptimizer/FunctionSignatureTransforms/FunctionSignatureOpts.h

@@ -65,14 +65,14 @@ struct ArgumentDescriptor {
   /// Is this parameter an indirect result?
   bool IsIndirectResult;
 
-  /// If non-null, this is the release in the return block of the callee, which
-  /// is associated with this parameter if it is @owned. If the parameter is not
-  /// @owned or we could not find such a release in the callee, this is null.
-  ReleaseList CalleeRelease;
-
-  /// The same as CalleeRelease, but the release in the throw block, if it is a
-  /// function which has a throw block.
-  ReleaseList CalleeReleaseInThrowBlock;
+  /// If non-empty, this is the set of releases in the return block of
+  /// the callee associated with this parameter if it is @owned. If it
+  /// is empty then we could not find any such releases.
+  TinyPtrVector<SILInstruction *> CalleeRelease;
+
+  /// The same as CalleeRelease, but the releases are post-dominated
+  /// by the throw block, if it is a function which has a throw block.
+  TinyPtrVector<SILInstruction *> CalleeReleaseInThrowBlock;
 
   /// The projection tree of this arguments.
   ProjectionTree ProjTree;

lib/SILOptimizer/FunctionSignatureTransforms/OwnedToGuaranteedTransform.cpp

@@ -92,8 +92,11 @@ bool FunctionSignatureTransform::OwnedToGuaranteedAnalyzeParameters() {
         auto ReleasesInThrow =
             ArgToThrowReleaseMap.getReleasesForArgument(A.Arg);
         if (!ArgToThrowReleaseMap.hasBlock() || !ReleasesInThrow.empty()) {
-          A.CalleeRelease = Releases;
-          A.CalleeReleaseInThrowBlock = ReleasesInThrow;
+          assert(A.CalleeRelease.empty());
+          assert(A.CalleeReleaseInThrowBlock.empty());
+          copy(Releases, std::back_inserter(A.CalleeRelease));
+          copy(ReleasesInThrow,
+               std::back_inserter(A.CalleeReleaseInThrowBlock));
           // We can convert this parameter to a @guaranteed.
           A.OwnedToGuaranteed = true;
           SignatureOptimize = true;