Merge pull request swiftlang#35583 from eeckstein/use-basicblockflag

Author: swift-ci

include/swift/SIL/BasicBlockUtils.h

@@ -14,6 +14,7 @@
 #define SWIFT_SIL_BASICBLOCKUTILS_H
 
 #include "swift/SIL/SILValue.h"
+#include "swift/SIL/SILBitfield.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
@@ -98,13 +99,6 @@ struct JointPostDominanceSetComputer {
   /// The worklist that drives the algorithm.
   SmallVector<SILBasicBlock *, 32> worklist;
 
-  /// A set that guards our worklist. Any block before it is added to worklist
-  /// should be checked against visitedBlocks.
-  SmallPtrSet<SILBasicBlock *, 32> visitedBlocks;
-
-  /// The set of blocks where we begin our walk.
-  SmallPtrSet<SILBasicBlock *, 8> initialBlocks;
-
   /// A subset of our initial blocks that we found as a predecessor of another
   /// block along our walk.
   SmallVector<SILBasicBlock *, 8> reachableInputBlocks;
@@ -113,7 +107,7 @@ struct JointPostDominanceSetComputer {
   /// visited yet are placed in here. At the end of our worklist, any blocks
   /// that remain here are "leaking blocks" that together with our initial set
   /// would provide a jointly-postdominating set of our dominating value.
-  SmallSetVector<SILBasicBlock *, 8> blocksThatLeakIfNeverVisited;
+  SmallVector<SILBasicBlock *, 32> blocksThatLeakIfNeverVisited;
 
   DeadEndBlocks &deadEndBlocks;
 
@@ -122,8 +116,6 @@ struct JointPostDominanceSetComputer {
 
   void clear() {
     worklist.clear();
-    visitedBlocks.clear();
-    initialBlocks.clear();
     reachableInputBlocks.clear();
     blocksThatLeakIfNeverVisited.clear();
   }

include/swift/SIL/LinearLifetimeChecker.h

@@ -20,6 +20,8 @@
 #include "swift/SIL/SILBasicBlock.h"
 #include "swift/SIL/SILFunction.h"
 #include "swift/SIL/SILValue.h"
+#include "swift/SIL/BasicBlockUtils.h"
+#include "swift/SIL/SILBitfield.h"
 #include "llvm/ADT/SmallPtrSet.h"
 
 namespace swift {
@@ -28,7 +30,6 @@ class SILBasicBlock;
 class SILInstruction;
 class SILModule;
 class SILValue;
-class DeadEndBlocks;
 class SILOwnershipVerifier;
 class SILValueOwnershipChecker;
 
@@ -56,13 +57,11 @@ class LinearLifetimeChecker {
   friend class SILOwnershipVerifier;
   friend class SILValueOwnershipChecker;
 
-  SmallPtrSetImpl<SILBasicBlock *> &visitedBlocks;
   DeadEndBlocks &deadEndBlocks;
 
 public:
-  LinearLifetimeChecker(SmallPtrSetImpl<SILBasicBlock *> &visitedBlocks,
-                        DeadEndBlocks &deadEndBlocks)
-      : visitedBlocks(visitedBlocks), deadEndBlocks(deadEndBlocks) {}
+  LinearLifetimeChecker(DeadEndBlocks &deadEndBlocks)
+      : deadEndBlocks(deadEndBlocks) {}
 
   /// Returns true that \p value forms a linear lifetime with consuming uses \p
   /// consumingUses, non consuming uses \p nonConsumingUses. Returns false

include/swift/SIL/OwnershipUtils.h

@@ -448,7 +448,6 @@ struct BorrowedValue {
   /// borrow scopes if needed.
   bool areUsesWithinScope(ArrayRef<Operand *> uses,
                           SmallVectorImpl<Operand *> &scratchSpace,
-                          SmallPtrSetImpl<SILBasicBlock *> &visitedBlocks,
                           DeadEndBlocks &deadEndBlocks) const;
 
   /// Given a local borrow scope introducer, visit all non-forwarding consuming

include/swift/SIL/SILBitfield.h

@@ -18,24 +18,29 @@
 #define SWIFT_SIL_SILBITFIELD_H
 
 #include "swift/SIL/SILFunction.h"
+#include "llvm/ADT/SmallVector.h"
 
 namespace swift {
 
 /// Utility to add a custom bitfield to a function's basic blocks.
 ///
 /// This can be used by transforms to store temporary flags or tiny values per
 /// basic block.
-/// It is very efficient: no memory allocation is needed, no hash set or map is
-/// needed for lookup and there is no initialization cost (in contrast to
-/// BasicBlockData which needs to iterate over all blocks at initialization).
+/// The memory managed is a 32 bit field within each basic block (\see
+/// BasicBlock::customBits) and thus is very efficient: no memory allocation is
+/// needed, no hash set or map is needed for lookup and there is no
+/// initialization cost (in contrast to BasicBlockData which needs to iterate
+/// over all blocks at initialization).
 ///
-/// Restrictions:
+/// Invariants:
 /// * BasicBlockBitfield instances must be allocated and deallocated
-///   following a strict stack discipline. This means, it's fine to use them as
-///   (or in) local variables in transformations. But it's e.g. not possible to
-///   store a BasicBlockBitfield in an Analysis.
+///   following a strict stack discipline, because bit-positions in
+///   BasicBlock::customBits are "allocated" and "freed" with a stack-allocation
+///   algorithm. This means, it's fine to use a BasicBlockBitfield as (or in)
+///   local variables, e.g. in transformations. But it's not possible to store
+///   a BasicBlockBitfield in an Analysis.
 /// * The total number of bits which are alive at the same time must not exceed
-///   32.
+///   32 (the size of BasicBlock::customBits).
 class BasicBlockBitfield {
   /// The bitfield is "added" to the blocks of this function.
   SILFunction *function;
@@ -89,6 +94,7 @@ class BasicBlockBitfield {
   SILFunction *getFunction() const { return function; }
 
   unsigned get(SILBasicBlock *block) const {
+    assert(block->getParent() == function);
     if (bitfieldID > block->lastInitializedBitfieldID) {
       // The bitfield is not initialized yet in this block.
       return 0;
@@ -97,6 +103,7 @@ class BasicBlockBitfield {
   }
 
   void set(SILBasicBlock *block, unsigned value) {
+    assert(block->getParent() == function);
     assert(((value << startBit) & ~mask) == 0 &&
            "value too large for BasicBlockBitfield");
     unsigned clearMask = mask;
@@ -136,7 +143,9 @@ class BasicBlockFlag {
 
   bool get(SILBasicBlock *block) const { return (bool)bit.get(block); }
 
-  void set(SILBasicBlock *block) { bit.set(block, 1); }
+  void set(SILBasicBlock *block, bool value = true) {
+    bit.set(block, (unsigned)value);
+  }
   void reset(SILBasicBlock *block) { bit.set(block, 0); }
 
   /// Sets the flag and returns the old value.
@@ -161,7 +170,45 @@ class BasicBlockSet {
   /// Returns true if \p block was not contained in the set before inserting.
   bool insert(SILBasicBlock *block) { return !flag.testAndSet(block); }
 
-  void remove(SILBasicBlock *block) { flag.reset(block); }
+  void erase(SILBasicBlock *block) { flag.reset(block); }
+};
+
+/// An implementation of `llvm::SetVector<SILBasicBlock *,
+///                                       SmallVector<SILBasicBlock *, N>,
+///                                       BasicBlockSet>`.
+///
+/// Unfortunately it's not possible to use `llvm::SetVector` directly because
+/// the BasicBlockSet constructor needs a `SILFunction` argument.
+///
+/// Note: This class does not provide a `remove` method intentinally, because
+/// it would have a O(n) complexity.
+template <unsigned N> class BasicBlockSetVector {
+  using Vector = llvm::SmallVector<SILBasicBlock *, N>;
+
+  Vector vector;
+  BasicBlockSet set;
+  
+public:
+  using iterator = typename Vector::const_iterator;
+
+  BasicBlockSetVector(SILFunction *function) : set(function) {}
+
+  iterator begin() const { return vector.begin(); }
+  iterator end() const { return vector.end(); }
+
+  unsigned size() const { return vector.size(); }
+  bool empty() const { return vector.empty(); }
+
+  bool contains(SILBasicBlock *block) const { return set.contains(block); }
+
+  /// Returns true if \p block was not contained in the set before inserting.
+  bool insert(SILBasicBlock *block) {
+    if (set.insert(block)) {
+      vector.push_back(block);
+      return true;
+    }
+    return false;
+  }
 };
 
 } // namespace swift

include/swift/SIL/SILCloner.h

@@ -67,9 +67,6 @@ class SILCloner : protected SILInstructionVisitor<ImplClass> {
   // convenient way to iterate over the cloned region.
   SmallVector<SILBasicBlock *, 8> preorderBlocks;
 
-  /// 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;
@@ -97,7 +94,6 @@ class SILCloner : protected SILInstructionVisitor<ImplClass> {
     ValueMap.clear();
     BBMap.clear();
     preorderBlocks.clear();
-    BlocksWithUnreachables.clear();
   }
 
   /// Clients of SILCloner who want to know about any newly created
@@ -170,12 +166,6 @@ class SILCloner : protected SILInstructionVisitor<ImplClass> {
     asImpl().mapValue(origValue, mappedValue);
   }
 
-  /// Mark a block containing an unreachable instruction for use in the `fixUp`
-  /// callback.
-  void addBlockWithUnreachable(SILBasicBlock *BB) {
-    BlocksWithUnreachables.insert(BB);
-  }
-
   /// Register a re-mapping for opened existentials.
   void registerOpenedExistentialRemapping(ArchetypeType *From,
                                           ArchetypeType *To) {
@@ -787,32 +777,6 @@ SILCloner<ImplClass>::doFixUp(SILFunction *F) {
     }
   }
 
-  // Remove any code after unreachable instructions.
-
-  // NOTE: It is unfortunate that it essentially duplicates the code from
-  // sil-combine, but doing so allows for avoiding any cross-layer invocations
-  // between SIL and SILOptimizer layers.
-
-  for (auto *BB : BlocksWithUnreachables) {
-    for (auto &I : *BB) {
-      if (!isa<UnreachableInst>(&I))
-        continue;
-
-      // Collect together all the instructions after this point
-      llvm::SmallVector<SILInstruction *, 32> ToRemove;
-      for (auto Inst = BB->rbegin(); &*Inst != &I; ++Inst)
-        ToRemove.push_back(&*Inst);
-
-      for (auto *Inst : ToRemove) {
-        // Replace any non-dead results with SILUndef values
-        Inst->replaceAllUsesOfAllResultsWithUndef();
-        Inst->eraseFromParent();
-      }
-    }
-  }
-
-  BlocksWithUnreachables.clear();
-
   // Call any cleanup specific to the CRTP extensions.
   asImpl().fixUp(F);
 }

include/swift/SIL/SILFunction.h

@@ -199,7 +199,7 @@ class SILFunction
 
   /// A monotonically increasing ID which is incremented whenever a
   /// BasicBlockBitfield is constructed.
-  /// Usually this stays below 1000, so a 32-bit unsigned is more than
+  /// Usually this stays below 100000, so a 32-bit unsigned is more than
   /// sufficient.
   /// For details see BasicBlockBitfield::bitfieldID;
   unsigned currentBitfieldID = 1;

include/swift/SIL/TypeSubstCloner.h

@@ -137,7 +137,6 @@ class TypeSubstCloner : public SILClonerWithScopes<ImplClass> {
   using SILClonerWithScopes<ImplClass>::getOpBasicBlock;
   using SILClonerWithScopes<ImplClass>::recordClonedInstruction;
   using SILClonerWithScopes<ImplClass>::recordFoldedValue;
-  using SILClonerWithScopes<ImplClass>::addBlockWithUnreachable;
   using SILClonerWithScopes<ImplClass>::OpenedArchetypesTracker;
 
   TypeSubstCloner(SILFunction &To,

lib/IRGen/IRGenSIL.cpp

@@ -31,6 +31,7 @@
 #include "swift/SIL/InstructionUtils.h"
 #include "swift/SIL/MemAccessUtils.h"
 #include "swift/SIL/PrettyStackTrace.h"
+#include "swift/SIL/SILBitfield.h"
 #include "swift/SIL/SILDebugScope.h"
 #include "swift/SIL/SILDeclRef.h"
 #include "swift/SIL/SILLinkage.h"
@@ -2007,7 +2008,7 @@ void IRGenSILFunction::emitSILFunction() {
 
   // Invariant: for every block in the work queue, we have visited all
   // of its dominators.
-  llvm::SmallPtrSet<SILBasicBlock*, 8> visitedBlocks;
+  BasicBlockSet visitedBlocks(CurSILFn);
   SmallVector<SILBasicBlock*, 8> workQueue; // really a stack
 
   // Queue up the entry block, for which the invariant trivially holds.
@@ -2042,15 +2043,15 @@ void IRGenSILFunction::emitSILFunction() {
     // Therefore the invariant holds of all the successors, and we can
     // queue them up if we haven't already visited them.
     for (auto *succBB : bb->getSuccessorBlocks()) {
-      if (visitedBlocks.insert(succBB).second)
+      if (visitedBlocks.insert(succBB))
         workQueue.push_back(succBB);
     }
   }
 
   // If there are dead blocks in the SIL function, we might have left
   // invalid blocks in the IR.  Do another pass and kill them off.
   for (SILBasicBlock &bb : *CurSILFn)
-    if (!visitedBlocks.count(&bb))
+    if (!visitedBlocks.contains(&bb))
       LoweredBBs[&bb].bb->eraseFromParent();
 
 }

lib/SIL/IR/SILFunction.cpp

@@ -10,6 +10,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+#define DEBUG_TYPE "sil-function"
+
 #include "swift/SIL/SILArgument.h"
 #include "swift/SIL/SILBasicBlock.h"
 #include "swift/SIL/SILFunction.h"
@@ -24,13 +26,16 @@
 #include "swift/Basic/OptimizationMode.h"
 #include "swift/Basic/Statistic.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/Statistic.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/GraphWriter.h"
 #include "clang/AST/Decl.h"
 
 using namespace swift;
 using namespace Lowering;
 
+STATISTIC(MaxBitfieldID, "Max value of SILFunction::currentBitfieldID");
+
 SILSpecializeAttr::SILSpecializeAttr(bool exported, SpecializationKind kind,
                                      GenericSignature specializedSig,
                                      SILFunction *target, Identifier spiGroup,
@@ -212,6 +217,8 @@ SILFunction::~SILFunction() {
          "Function cannot be deleted while function_ref's still exist");
   assert(!newestAliveBitfield &&
          "Not all BasicBlockBitfields deleted at function destruction");
+  if (currentBitfieldID > MaxBitfieldID)
+    MaxBitfieldID = currentBitfieldID;
 }
 
 void SILFunction::createProfiler(ASTNode Root, SILDeclRef forDecl,