---

yaml
---
r: 346029
b: refs/heads/master
c: b7e10c1
h: refs/heads/master
i:
  346027: a72188e

Author: swift-ci

[refs]

@@ -1,5 +1,5 @@
 ---
-refs/heads/master: c14c5c9af346299cfaecc2ceba85c1406c002a38
+refs/heads/master: b7e10c1b676b2fc4fdf7ef010c68eda8f9cbdc8f
 refs/heads/master-next: 203b3026584ecad859eb328b2e12490099409cd5
 refs/tags/osx-passed: b6b74147ef8a386f532cf9357a1bde006e552c54
 refs/tags/swift-2.2-SNAPSHOT-2015-12-01-a: 6bb18e013c2284f2b45f5f84f2df2887dc0f7dea

trunk/include/swift/Basic/Statistic.h

@@ -21,7 +21,9 @@
 #include <thread>
 #include <tuple>
 
-#define SWIFT_FUNC_STAT                                                 \
+#define SWIFT_FUNC_STAT SWIFT_FUNC_STAT_NAMED(DEBUG_TYPE)
+
+#define SWIFT_FUNC_STAT_NAMED(DEBUG_TYPE)                               \
   do {                                                                  \
     static llvm::Statistic FStat =                                      \
       {DEBUG_TYPE, __func__, __func__, {0}, {false}};                   \

trunk/include/swift/SIL/SILCloner.h

@@ -67,6 +67,10 @@ class SILCloner : protected SILInstructionVisitor<ImplClass> {
   /// Set of basic blocks where unreachable was inserted.
   SmallPtrSet<SILBasicBlock *, 32> BlocksWithUnreachables;
 
+  // Keep track of the last cloned block in function order. For single block
+  // regions, this will be the start block.
+  SILBasicBlock *lastClonedBB = nullptr;
+
 public:
   using SILInstructionVisitor<ImplClass>::asImpl;
 
@@ -102,6 +106,10 @@ class SILCloner : protected SILInstructionVisitor<ImplClass> {
 
   SILBuilder &getBuilder() { return Builder; }
 
+  // After cloning, returns a non-null pointer to the last cloned block in
+  // function order. For single block regions, this will be the start block.
+  SILBasicBlock *getLastClonedBB() { return lastClonedBB; }
+
   /// Visit all blocks reachable from the given `StartBB` and all instructions
   /// in those blocks.
   ///
@@ -655,7 +663,7 @@ void SILCloner<ImplClass>::visitBlocksDepthFirst(SILBasicBlock *startBB) {
   // order they are created, which differs from the order they are
   // cloned. Blocks are created in BFS order but cloned in DFS preorder (when no
   // critical edges are present).
-  SILBasicBlock *lastClonedBB = BBMap[startBB];
+  lastClonedBB = BBMap[startBB];
   while (!dfsWorklist.empty()) {
     auto *BB = dfsWorklist.pop_back_val();
     preorderBlocks.push_back(BB);

trunk/include/swift/SILOptimizer/Utils/CFG.h

@@ -127,6 +127,16 @@ bool splitAllCondBrCriticalEdgesWithNonTrivialArgs(SILFunction &Fn,
 bool mergeBasicBlockWithSuccessor(SILBasicBlock *BB, DominanceInfo *DT,
                                   SILLoopInfo *LI);
 
+/// Merge basic blocks in the given function by eliminating all unconditional
+/// branches to single-predecessor branch targets.
+///
+/// During optimization, SimplifyCFG also handles this, but this is a basic
+/// canonicalization after any pass that splits blocks, such as inlining. This
+/// is not done on-the-fly after splitting blocks because merging is linear in
+/// the number of instructions, so interleaved merging and splitting is
+/// quadratic.
+bool mergeBasicBlocks(SILFunction *F);
+
 /// Given a list of \p UserBlocks and a list of \p DefBlocks, find a set of
 /// blocks that together with \p UserBlocks joint-postdominate \p
 /// DefBlocks. This is in a sense finding a set of blocks that "complete" \p

trunk/include/swift/SILOptimizer/Utils/SILInliner.h

@@ -80,18 +80,32 @@ class SILInliner {
   /// iterator to the first inlined instruction (or first instruction after the
   /// call for an empty function).
   ///
-  /// This may split basic blocks and delete instructions.
-  ///
   /// This only performs one step of inlining: it does not recursively
   /// inline functions called by the callee.
   ///
+  /// This may split basic blocks and delete instructions anywhere.
+  ///
+  /// All inlined instructions must be either inside the original call block or
+  /// inside new basic blocks laid out after the original call block.
+  ///
+  /// Any instructions in the original call block after the inlined call must be
+  /// in a new basic block laid out after all inlined blocks.
+  ///
+  /// The above guarantees ensure that inlining is liner in the number of
+  /// instructions and that inlined instructions are revisited exactly once.
+  ///
   /// *NOTE*: This attempts to perform inlining unconditionally and thus asserts
   /// if inlining will fail. All users /must/ check that a function is allowed
   /// to be inlined using SILInliner::canInlineApplySite before calling this
   /// function.
-  SILBasicBlock::iterator inlineFunction(SILFunction *calleeFunction,
-                                         FullApplySite apply,
-                                         ArrayRef<SILValue> appliedArgs);
+  ///
+  /// Returns an iterator to the first inlined instruction (or the end of the
+  /// caller block for empty functions) and the last block in function order
+  /// containing inlined instructions (the original caller block for
+  /// single-block functions).
+  std::pair<SILBasicBlock::iterator, SILBasicBlock *>
+  inlineFunction(SILFunction *calleeFunction, FullApplySite apply,
+                 ArrayRef<SILValue> appliedArgs);
 };
 
 } // end namespace swift

trunk/lib/SIL/SILInstruction.cpp

@@ -86,8 +86,10 @@ transferNodesFromList(llvm::ilist_traits<SILInstruction> &L2,
   if (ThisParent == L2.getContainingBlock()) return;
 
   // Update the parent fields in the instructions.
-  for (; first != last; ++first)
+  for (; first != last; ++first) {
+    SWIFT_FUNC_STAT_NAMED("sil");
     first->ParentBB = ThisParent;
+  }
 }
 
 //===----------------------------------------------------------------------===//

trunk/lib/SILOptimizer/Mandatory/MandatoryInlining.cpp

@@ -49,7 +49,7 @@ static void diagnose(ASTContext &Context, SourceLoc loc, Diag<T...> diag,
 /// and that the function implementing the closure consumes its capture
 /// arguments.
 static void fixupReferenceCounts(
-    SILBasicBlock::iterator I, SILValue CalleeValue,
+    SILInstruction *I, SILValue CalleeValue,
     SmallVectorImpl<std::pair<SILValue, ParameterConvention>> &CaptureArgs,
     bool isCalleeGuaranteed) {
   // Add a copy of each non-address type capture argument to lifetime extend the
@@ -60,7 +60,7 @@ static void fixupReferenceCounts(
     if (!CaptureArg.first->getType().isAddress() &&
         CaptureArg.second != ParameterConvention::Direct_Guaranteed &&
         CaptureArg.second != ParameterConvention::Direct_Unowned) {
-      createIncrementBefore(CaptureArg.first, &*I);
+      createIncrementBefore(CaptureArg.first, I);
     } else {
       // FIXME: What about indirectly owned parameters? The invocation of the
       // closure would perform an indirect copy which we should mimick here.
@@ -190,35 +190,31 @@ namespace {
 class ClosureCleanup {
   using DeadInstSet = SmallBlotSetVector<SILInstruction *, 4>;
 
-  /// A helper class to update an instruction iterator if
-  /// removal of instructions would invalidate it.
+  /// A helper class to update the set of dead instructions.
   ///
   /// Since this is called by the SILModule callback, the instruction may longer
   /// be well-formed. Do not visit its operands. However, it's position in the
   /// basic block is still valid.
   ///
-  /// FIXME: Using the Module's callback mechanism is undesirable. Instead,
-  /// cleanupCalleeValue could be easily rewritten to use its own instruction
-  /// deletion helper and pass a callback to tryDeleteDeadClosure and
-  /// recursivelyDeleteTriviallyDeadInstructions.
-  class IteratorUpdateHandler : public DeleteNotificationHandler {
+  /// FIXME: Using the Module's callback mechanism for this is terrible.
+  /// Instead, cleanupCalleeValue could be easily rewritten to use its own
+  /// instruction deletion helper and pass a callback to tryDeleteDeadClosure
+  /// and recursivelyDeleteTriviallyDeadInstructions.
+  class DeleteUpdateHandler : public DeleteNotificationHandler {
     SILModule &Module;
-    SILBasicBlock::iterator CurrentI;
     DeadInstSet &DeadInsts;
 
   public:
-    IteratorUpdateHandler(SILBasicBlock::iterator I, DeadInstSet &DeadInsts)
-        : Module(I->getModule()), CurrentI(I), DeadInsts(DeadInsts) {
+    DeleteUpdateHandler(SILModule &M, DeadInstSet &DeadInsts)
+        : Module(M), DeadInsts(DeadInsts) {
       Module.registerDeleteNotificationHandler(this);
     }
 
-    ~IteratorUpdateHandler() override {
+    ~DeleteUpdateHandler() override {
       // Unregister the handler.
       Module.removeDeleteNotificationHandler(this);
     }
 
-    SILBasicBlock::iterator getIterator() const { return CurrentI; }
-
     // Handling of instruction removal notifications.
     bool needsNotifications() override { return true; }
 
@@ -229,9 +225,6 @@ class ClosureCleanup {
         return;
 
       DeadInsts.erase(deletedI);
-
-      if (CurrentI == SILBasicBlock::iterator(deletedI))
-        ++CurrentI;
     }
   };
 
@@ -260,16 +253,15 @@ class ClosureCleanup {
   // Note: instructions in the `deadFunctionVals` set may use each other, so the
   // set needs to continue to be updated (by this handler) when deleting
   // instructions. This assumes that DeadFunctionValSet::erase() is stable.
-  SILBasicBlock::iterator cleanupDeadClosures(SILBasicBlock::iterator II) {
-    IteratorUpdateHandler iteratorUpdate(II, deadFunctionVals);
+  void cleanupDeadClosures(SILFunction *F) {
+    DeleteUpdateHandler deleteUpdate(F->getModule(), deadFunctionVals);
     for (Optional<SILInstruction *> I : deadFunctionVals) {
       if (!I.hasValue())
         continue;
 
       if (auto *SVI = dyn_cast<SingleValueInstruction>(I.getValue()))
         cleanupCalleeValue(SVI);
     }
-    return iteratorUpdate.getIterator();
   }
 };
 } // end of namespace
@@ -472,21 +464,21 @@ static SILFunction *getCalleeFunction(
   return CalleeFunction;
 }
 
-static std::tuple<FullApplySite, SILBasicBlock::iterator>
-tryDevirtualizeApplyHelper(FullApplySite InnerAI, SILBasicBlock::iterator I,
-                           ClassHierarchyAnalysis *CHA) {
+static SILInstruction *tryDevirtualizeApplyHelper(FullApplySite InnerAI,
+                                                  ClassHierarchyAnalysis *CHA) {
   auto NewInst = tryDevirtualizeApply(InnerAI, CHA);
-  if (!NewInst) {
-    return std::make_tuple(InnerAI, I);
-  }
+  if (!NewInst)
+    return InnerAI.getInstruction();
 
   deleteDevirtualizedApply(InnerAI);
 
+  // FIXME: Comments at the use of this helper indicate that devirtualization
+  // may return SILArgument. Yet here we assert that it must return an
+  // instruction.
   auto newApplyAI = NewInst.getInstruction();
   assert(newApplyAI && "devirtualized but removed apply site?");
 
-  return std::make_tuple(FullApplySite::isa(newApplyAI),
-                         newApplyAI->getIterator());
+  return newApplyAI;
 }
 
 /// \brief Inlines all mandatory inlined functions into the body of a function,
@@ -532,23 +524,34 @@ runOnFunctionRecursively(SILOptFunctionBuilder &FuncBuilder,
   SmallVector<std::pair<SILValue, ParameterConvention>, 16> CaptureArgs;
   SmallVector<SILValue, 32> FullArgs;
 
-  for (auto BI = F->begin(), BE = F->end(); BI != BE; ++BI) {
-    // While iterating over this block, instructions are inserted and deleted.
-    for (auto II = BI->begin(), nextI = II; II != BI->end(); II = nextI) {
-      nextI = std::next(II);
+  // Visiting blocks in reverse order avoids revisiting instructions after block
+  // splitting, which would be quadratic.
+  for (auto BI = F->rbegin(), BE = F->rend(), nextBB = BI; BI != BE;
+       BI = nextBB) {
+    // After inlining, the block iterator will be adjusted to point to the last
+    // block containing inlined instructions. This way, the inlined function
+    // body will be reprocessed within the caller's context without revisiting
+    // any original instructions.
+    nextBB = std::next(BI);
 
+    // While iterating over this block, instructions are inserted and deleted.
+    // To avoid quadratic block splitting, instructions must be processed in
+    // reverse order (block splitting reassigned the parent pointer of all
+    // instructions below the split point).
+    for (auto II = BI->rbegin(); II != BI->rend(); ++II) {
       FullApplySite InnerAI = FullApplySite::isa(&*II);
-
       if (!InnerAI)
         continue;
 
-      auto *ApplyBlock = InnerAI.getParent();
-
-      // *NOTE* If devirtualization succeeds, sometimes II will not be InnerAI,
+      // *NOTE* If devirtualization succeeds, devirtInst may not be InnerAI,
       // but a casted result of InnerAI or even a block argument due to
-      // abstraction changes when calling the witness or class method. We still
-      // know that InnerAI dominates II though.
-      std::tie(InnerAI, II) = tryDevirtualizeApplyHelper(InnerAI, II, CHA);
+      // abstraction changes when calling the witness or class method.
+      auto *devirtInst = tryDevirtualizeApplyHelper(InnerAI, CHA);
+      // Restore II to the current apply site.
+      II = devirtInst->getReverseIterator();
+      // If the devirtualized call result is no longer a invalid FullApplySite,
+      // then it has succeeded, but the result is not immediately inlinable.
+      InnerAI = FullApplySite::isa(devirtInst);
       if (!InnerAI)
         continue;
 
@@ -597,13 +600,8 @@ runOnFunctionRecursively(SILOptFunctionBuilder &FuncBuilder,
 
       SILInliner Inliner(FuncBuilder, SILInliner::InlineKind::MandatoryInline,
                          Subs, OpenedArchetypesTracker);
-      if (!Inliner.canInlineApplySite(InnerAI)) {
-        // See comment above about casting when devirtualizing and how this
-        // sometimes causes II and InnerAI to be different and even in different
-        // blocks.
-        II = InnerAI.getInstruction()->getIterator();
+      if (!Inliner.canInlineApplySite(InnerAI))
         continue;
-      }
 
       // Inline function at I, which also changes I to refer to the first
       // instruction inlined in the case that it succeeds. We purposely
@@ -620,7 +618,8 @@ runOnFunctionRecursively(SILOptFunctionBuilder &FuncBuilder,
         bool IsCalleeGuaranteed =
             PAI &&
             PAI->getType().castTo<SILFunctionType>()->isCalleeGuaranteed();
-        fixupReferenceCounts(II, CalleeValue, CaptureArgs, IsCalleeGuaranteed);
+        fixupReferenceCounts(InnerAI.getInstruction(), CalleeValue, CaptureArgs,
+                             IsCalleeGuaranteed);
       }
 
       // Register a callback to record potentially unused function values after
@@ -632,20 +631,23 @@ runOnFunctionRecursively(SILOptFunctionBuilder &FuncBuilder,
 
       // Inlining deletes the apply, and can introduce multiple new basic
       // blocks. After this, CalleeValue and other instructions may be invalid.
-      nextI = Inliner.inlineFunction(CalleeFunction, InnerAI, FullArgs);
+      // nextBB will point to the last inlined block
+      auto firstInlinedInstAndLastBB =
+          Inliner.inlineFunction(CalleeFunction, InnerAI, FullArgs);
+      nextBB = firstInlinedInstAndLastBB.second->getReverseIterator();
       ++NumMandatoryInlines;
 
       // The IR is now valid, and trivial dead arguments are removed. However,
       // we may be able to remove dead callee computations (e.g. dead
       // partial_apply closures).
-      nextI = closureCleanup.cleanupDeadClosures(nextI);
+      closureCleanup.cleanupDeadClosures(F);
 
-      assert(nextI == ApplyBlock->end()
-             || nextI->getParent() == ApplyBlock
-                    && "Mismatch between the instruction and basic block");
+      // Resume inlining within nextBB, which contains only the inlined
+      // instructions and possibly instructions in the original call block that
+      // have not yet been visited.
+      break;
     }
   }
-
   // Keep track of full inlined functions so we don't waste time recursively
   // reprocessing them.
   FullyInlinedSet.insert(F);
@@ -680,6 +682,9 @@ class MandatoryInlining : public SILModuleTransform {
       runOnFunctionRecursively(FuncBuilder, &F,
                                FullApplySite(), FullyInlinedSet, SetFactory,
                                SetFactory.getEmptySet(), CHA);
+      // The inliner splits blocks at call sites. Re-merge trivial branches
+      // to reestablish a canonical CFG.
+      mergeBasicBlocks(&F);
     }
 
     if (!ShouldCleanup)

trunk/lib/SILOptimizer/Transforms/PerformanceInliner.cpp

@@ -17,6 +17,7 @@
 #include "swift/SILOptimizer/Analysis/SideEffectAnalysis.h"
 #include "swift/SILOptimizer/PassManager/Passes.h"
 #include "swift/SILOptimizer/PassManager/Transforms.h"
+#include "swift/SILOptimizer/Utils/CFG.h"
 #include "swift/SILOptimizer/Utils/Devirtualize.h"
 #include "swift/SILOptimizer/Utils/Generics.h"
 #include "swift/SILOptimizer/Utils/PerformanceInlinerUtils.h"
@@ -903,6 +904,9 @@ bool SILPerformanceInliner::inlineCallsIntoFunction(SILFunction *Caller) {
     Inliner.inlineFunction(Callee, AI, Args);
     NumFunctionsInlined++;
   }
+  // The inliner splits blocks at call sites. Re-merge trivial branches to
+  // reestablish a canonical CFG.
+  mergeBasicBlocks(Caller);
   return true;
 }
 

trunk/lib/SILOptimizer/Utils/CFG.cpp

@@ -493,6 +493,19 @@ bool swift::mergeBasicBlockWithSuccessor(SILBasicBlock *BB, DominanceInfo *DT,
   return true;
 }
 
+bool swift::mergeBasicBlocks(SILFunction *F) {
+  bool merged = false;
+  for (auto BBIter = F->begin(); BBIter != F->end();) {
+    if (mergeBasicBlockWithSuccessor(&*BBIter, /*DT*/ nullptr, /*LI*/ nullptr)) {
+      merged = true;
+      // Continue to merge the current block without advancing.
+      continue;
+    }
+    ++BBIter;
+  }
+  return merged;
+}
+
 /// Splits the critical edges between from and to. This code assumes there is
 /// only one edge between the two basic blocks.
 SILBasicBlock *swift::splitIfCriticalEdge(SILBasicBlock *From,