[BOLT][Refactoring] Change landing pads handling

Summary: Change the way we store and handle landing pads and throwers.

(cherry picked from FBD6169992)

Author: maksfb

bolt/BinaryBasicBlock.cpp

@@ -293,22 +293,6 @@ void BinaryBasicBlock::removeDuplicateConditionalSuccessor(MCInst *CondBranch) {
   BranchInfo.push_back({Count, 0});
 }
 
-void BinaryBasicBlock::addLandingPad(BinaryBasicBlock *LPBlock) {
-  if (std::find(LandingPads.begin(), LandingPads.end(), LPBlock) == LandingPads.end()) {
-    LandingPads.push_back(LPBlock);
-  }
-  LPBlock->Throwers.insert(this);
-}
-
-void BinaryBasicBlock::clearLandingPads() {
-  for (auto *LPBlock : LandingPads) {
-    auto Count = LPBlock->Throwers.erase(this);
-    (void)Count;
-    assert(Count == 1 && "Possible duplicate entry in LandingPads");
-  }
-  LandingPads.clear();
-}
-
 bool BinaryBasicBlock::analyzeBranch(const MCSymbol *&TBB,
                                      const MCSymbol *&FBB,
                                      MCInst *&CondBranch,

bolt/BinaryBasicBlock.h

@@ -60,7 +60,7 @@ class BinaryBasicBlock {
   /// CFG information.
   std::vector<BinaryBasicBlock *> Predecessors;
   std::vector<BinaryBasicBlock *> Successors;
-  std::set<BinaryBasicBlock *> Throwers;
+  std::vector<BinaryBasicBlock *> Throwers;
   std::vector<BinaryBasicBlock *> LandingPads;
 
   /// Each successor has a corresponding BranchInfo entry in the list.
@@ -222,7 +222,7 @@ class BinaryBasicBlock {
     return (unsigned)Throwers.size();
   }
   bool                  throw_empty() const { return Throwers.empty(); }
-  bool                 isLandingPad() const { return !Throwers.empty(); }
+  bool                  isLandingPad() const { return !Throwers.empty(); }
 
   lp_iterator        lp_begin()       { return LandingPads.begin();   }
   const_lp_iterator  lp_begin() const { return LandingPads.begin();   }
@@ -524,9 +524,6 @@ class BinaryBasicBlock {
                         uint64_t Count = 0,
                         uint64_t MispredictedCount = 0);
 
-  /// Adds block to landing pad list.
-  void addLandingPad(BinaryBasicBlock *LPBlock);
-
   /// Remove /p Succ basic block from the list of successors. Update the
   /// list of predecessors of /p Succ and update branch info.
   void removeSuccessor(BinaryBasicBlock *Succ);
@@ -781,9 +778,6 @@ class BinaryBasicBlock {
   /// use removeSuccessor() function.
   void removePredecessor(BinaryBasicBlock *Pred);
 
-  /// Remove landing pads of this basic block.
-  void clearLandingPads();
-
   /// Return offset of the basic block from the function start.
   uint32_t getOffset() const {
     return InputRange.first;

bolt/BinaryFunction.cpp

@@ -330,10 +330,8 @@ std::pair<unsigned, uint64_t> BinaryFunction::eraseInvalidBBs() {
   assert(BasicBlocks.size() == BasicBlocksLayout.size());
 
   // Update CFG state if needed
-  if (Count > 0) {
-    updateBBIndices(0);
-    recomputeLandingPads(0, BasicBlocks.size());
-  }
+  if (Count > 0)
+    recomputeLandingPads();
 
   return std::make_pair(Count, Bytes);
 }
@@ -1457,54 +1455,39 @@ bool BinaryFunction::postProcessIndirectBranches() {
   return true;
 }
 
-void BinaryFunction::clearLandingPads(const unsigned StartIndex,
-                                      const unsigned NumBlocks) {
-  // remove all landing pads/throws for the given collection of blocks
-  for (auto I = StartIndex; I < StartIndex + NumBlocks; ++I) {
-    BasicBlocks[I]->clearLandingPads();
-  }
-}
+void BinaryFunction::recomputeLandingPads() {
+  updateBBIndices(0);
 
-void BinaryFunction::addLandingPads(const unsigned StartIndex,
-                                    const unsigned NumBlocks) {
   for (auto *BB : BasicBlocks) {
-    if (LandingPads.find(BB->getLabel()) != LandingPads.end()) {
-      const MCSymbol *LP = BB->getLabel();
-      for (unsigned I : LPToBBIndex[LP]) {
-        assert(I < BasicBlocks.size());
-        BinaryBasicBlock *ThrowBB = BasicBlocks[I];
-        const unsigned ThrowBBIndex = getIndex(ThrowBB);
-        if (ThrowBBIndex >= StartIndex && ThrowBBIndex < StartIndex + NumBlocks)
-          ThrowBB->addLandingPad(BB);
-      }
-    }
+    BB->LandingPads.clear();
+    BB->Throwers.clear();
   }
 
-  clearList(LPToBBIndex);
-}
+  for (auto *BB : BasicBlocks) {
+    for (auto &Instr : *BB) {
+      if (!BC.MIA->isInvoke(Instr))
+        continue;
 
-void BinaryFunction::recomputeLandingPads(const unsigned StartIndex,
-                                          const unsigned NumBlocks) {
-  assert(LPToBBIndex.empty());
-
-  clearLandingPads(StartIndex, NumBlocks);
-
-  for (auto I = StartIndex; I < StartIndex + NumBlocks; ++I) {
-    auto *BB = BasicBlocks[I];
-    for (auto &Instr : BB->instructions()) {
-      // Store info about associated landing pad.
-      if (BC.MIA->isInvoke(Instr)) {
-        const MCSymbol *LP;
-        uint64_t Action;
-        std::tie(LP, Action) = BC.MIA->getEHInfo(Instr);
-        if (LP) {
-          LPToBBIndex[LP].push_back(getIndex(BB));
-        }
-      }
+      const MCSymbol *LPLabel;
+      uint64_t Action;
+      std::tie(LPLabel, Action) = BC.MIA->getEHInfo(Instr);
+      if (!LPLabel)
+        continue;
+
+      auto *LPBlock = getBasicBlockForLabel(LPLabel);
+      BB->LandingPads.emplace_back(LPBlock);
+      LPBlock->Throwers.emplace_back(BB);
     }
+    std::sort(BB->lp_begin(), BB->lp_end());
+    auto NewEnd = std::unique(BB->lp_begin(), BB->lp_end());
+    BB->LandingPads.erase(NewEnd, BB->lp_end());
   }
 
-  addLandingPads(StartIndex, NumBlocks);
+  for (auto *BB : BasicBlocks) {
+    std::sort(BB->throw_begin(), BB->throw_end());
+    auto NewEnd = std::unique(BB->throw_begin(), BB->throw_end());
+    BB->Throwers.erase(NewEnd, BB->throw_end());
+  }
 }
 
 bool BinaryFunction::buildCFG() {
@@ -1608,16 +1591,6 @@ bool BinaryFunction::buildCFG() {
       CFIOffset = getSize();
     addCFIPlaceholders(CFIOffset, InsertBB);
 
-    // Store info about associated landing pad.
-    if (MIA->isInvoke(Instr)) {
-      const MCSymbol *LP;
-      uint64_t Action;
-      std::tie(LP, Action) = MIA->getEHInfo(Instr);
-      if (LP) {
-        LPToBBIndex[LP].push_back(getIndex(InsertBB));
-      }
-    }
-
     if (MIA->isTerminator(Instr)) {
       PrevBB = InsertBB;
       InsertBB = nullptr;
@@ -1810,8 +1783,7 @@ bool BinaryFunction::buildCFG() {
     DEBUG(dbgs() << "last block was marked as a fall-through\n");
   }
 
-  // Add associated landing pad blocks to each basic block.
-  addLandingPads(0, BasicBlocks.size());
+  recomputeLandingPads();
 
   // Infer frequency for non-taken branches
   if (hasValidProfile() && opts::DoMCF != MCF_DISABLE) {
@@ -1873,7 +1845,6 @@ bool BinaryFunction::buildCFG() {
   clearList(TakenBranches);
   clearList(FTBranches);
   clearList(IgnoredBranches);
-  clearList(LPToBBIndex);
   clearList(EntryOffsets);
 
   // Update the state.
@@ -3033,23 +3004,35 @@ bool BinaryFunction::validateCFG() const {
     return Valid;
 
   for (auto *BB : BasicBlocks) {
-    std::set<BinaryBasicBlock *> Seen;
+    if (!std::is_sorted(BB->lp_begin(), BB->lp_end())) {
+      errs() << "BOLT-ERROR: unsorted list of landing pads in "
+             << BB->getName() << " in function " << *this << '\n';
+      return false;
+    }
+    if (std::unique(BB->lp_begin(), BB->lp_end()) != BB->lp_end()) {
+      errs() << "BOLT-ERROR: duplicate landing pad detected in"
+             << BB->getName() << " in function " << *this << '\n';
+      return false;
+    }
+    if (!std::is_sorted(BB->throw_begin(), BB->throw_end())) {
+      errs() << "BOLT-ERROR: unsorted list of throwers in "
+             << BB->getName() << " in function " << *this << '\n';
+      return false;
+    }
+    if (std::unique(BB->throw_begin(), BB->throw_end()) != BB->throw_end()) {
+      errs() << "BOLT-ERROR: duplicate thrower detected in"
+             << BB->getName() << " in function " << *this << '\n';
+      return false;
+    }
     for (auto *LPBlock : BB->LandingPads) {
-      Valid &= Seen.count(LPBlock) == 0;
-      if (!Valid) {
-        errs() << "BOLT-WARNING: Duplicate LP seen " << LPBlock->getName()
-               << "in " << *this << "\n";
-        break;
-      }
-      Seen.insert(LPBlock);
-      auto count = LPBlock->Throwers.count(BB);
-      Valid &= (count == 1);
-      if (!Valid) {
-        errs() << "BOLT-WARNING: Inconsistent landing pad detected in "
-               << *this << ": " << LPBlock->getName()
-               << " is in LandingPads but not in " << BB->getName()
-               << "->Throwers\n";
-        break;
+      if (!std::binary_search(LPBlock->throw_begin(),
+                              LPBlock->throw_end(),
+                              BB)) {
+        errs() << "BOLT-ERROR: inconsistent landing pad detected in "
+               << *this << ": " << BB->getName()
+               << " is in LandingPads but not in " << LPBlock->getName()
+               << " Throwers\n";
+        return false;
       }
     }
   }
@@ -3590,12 +3573,7 @@ void BinaryFunction::insertBasicBlocks(
     BasicBlocks[I++] = BB.release();
   }
 
-  updateBBIndices(StartIndex);
-
-  recomputeLandingPads(StartIndex, NumNewBlocks + 1);
-
-  // Make sure the basic blocks are sorted properly.
-  assert(std::is_sorted(begin(), end()));
+  recomputeLandingPads();
 
   if (UpdateLayout) {
     updateLayout(Start, NumNewBlocks);
@@ -3624,12 +3602,7 @@ BinaryFunction::iterator BinaryFunction::insertBasicBlocks(
     BasicBlocks[I++] = BB.release();
   }
 
-  updateBBIndices(StartIndex);
-
-  recomputeLandingPads(StartIndex, NumNewBlocks + 1);
-
-  // Make sure the basic blocks are sorted properly.
-  assert(std::is_sorted(begin(), end()));
+  recomputeLandingPads();
 
   if (UpdateLayout) {
     updateLayout(*std::prev(RetIter), NumNewBlocks);

bolt/BinaryFunction.h

@@ -325,9 +325,6 @@ class BinaryFunction {
   /// Original LSDA address for the function.
   uint64_t LSDAAddress{0};
 
-  /// Landing pads for the function.
-  std::set<const MCSymbol *> LandingPads;
-
   /// Associated DIEs in the .debug_info section with their respective CUs.
   /// There can be multiple because of identical code folding.
   std::vector<std::pair<const DWARFDebugInfoEntryMinimal *,
@@ -441,19 +438,8 @@ class BinaryFunction {
     return InstA.equals(InstB, Comp);
   }
 
-  /// Clear the landing pads for all blocks contained in the range of
-  /// [StartIndex, StartIndex + NumBlocks).  This also has the effect of
-  /// removing throws that point to any of these blocks.
-  void clearLandingPads(const unsigned StartIndex, const unsigned NumBlocks);
-
-  /// Add landing pads for all blocks in the range
-  /// [StartIndex, StartIndex + NumBlocks) using LPToBBIndex.
-  void addLandingPads(const unsigned StartIndex, const unsigned NumBlocks);
-
-  /// Recompute the landing pad information for all the basic blocks in the
-  /// range of [StartIndex to StartIndex + NumBlocks).
-  void recomputeLandingPads(const unsigned StartIndex,
-                            const unsigned NumBlocks);
+  /// Recompute landing pad information for the function and all its blocks.
+  void recomputeLandingPads();
 
   /// Temporary holder of offsets that are potentially entry points.
   std::unordered_set<uint64_t> EntryOffsets;
@@ -466,10 +452,6 @@ class BinaryFunction {
   BranchListType FTBranches;          /// All fall-through branches.
   BranchListType IgnoredBranches;     /// Branches ignored by CFG purposes.
 
-  /// Storage for all landing pads and their corresponding invokes.
-  using LandingPadsMapType = std::map<const MCSymbol *, std::vector<unsigned> >;
-  LandingPadsMapType LPToBBIndex;
-
   /// Map offset in the function to a label.
   /// Labels are used for building CFG for simple functions. For non-simple
   /// function in relocation mode we need to emit them for relocations

bolt/Exceptions.cpp

@@ -233,7 +233,6 @@ void BinaryFunction::parseLSDA(ArrayRef<uint8_t> LSDASectionData,
           LPSymbol = BC.Ctx->createTempSymbol("LP", true);
           Labels[LandingPad] = LPSymbol;
         }
-        LandingPads.insert(LPSymbol);
       }
     }