Revert r272891 "[JumpThreading] Prevent dangling pointer problems in BranchProbabilityInfo"

It was causing failures in Profile-i386 and Profile-x86_64 tests.

llvm-svn: 272912

llvm/include/llvm/Analysis/BranchProbabilityInfo.h

@@ -18,7 +18,6 @@
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/PassManager.h"
-#include "llvm/IR/ValueHandle.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/BranchProbability.h"
@@ -117,16 +116,13 @@ public:
 
   void calculate(const Function &F, const LoopInfo &LI);
 
-  /// Forget analysis results for the given basic block.
-  void eraseBlock(const BasicBlock *BB);
-
 private:
   void operator=(const BranchProbabilityInfo &) = delete;
   BranchProbabilityInfo(const BranchProbabilityInfo &) = delete;
 
   // Since we allow duplicate edges from one basic block to another, we use
   // a pair (PredBlock and an index in the successors) to specify an edge.
-  typedef std::pair<AssertingVH<const BasicBlock>, unsigned> Edge;
+  typedef std::pair<const BasicBlock *, unsigned> Edge;
 
   // Default weight value. Used when we don't have information about the edge.
   // TODO: DEFAULT_WEIGHT makes sense during static predication, when none of

llvm/include/llvm/Transforms/Scalar/JumpThreading.h

@@ -134,8 +134,6 @@ private:
                               const char *Suffix);
   void UpdateBlockFreqAndEdgeWeight(BasicBlock *PredBB, BasicBlock *BB,
                                     BasicBlock *NewBB, BasicBlock *SuccBB);
-
-  void dropBlockAnalysisResults(BasicBlock *BB);
 };
 
 } // end namespace llvm

llvm/include/llvm/Transforms/Utils/Local.h

@@ -44,7 +44,6 @@ class TargetTransformInfo;
 class DIBuilder;
 class DominatorTree;
 class LazyValueInfo;
-class BranchProbabilityInfo;
 
 template<typename T> class SmallVectorImpl;
 
@@ -301,11 +300,7 @@ void removeUnwindEdge(BasicBlock *BB);
 /// Remove all blocks that can not be reached from the function's entry.
 ///
 /// Returns true if any basic block was removed.
-/// \param F Target function
-/// \param LVI Will update this analysis if non null
-/// \param BPI Will update this analysis if non null
-bool removeUnreachableBlocks(Function &F, LazyValueInfo *LVI = nullptr,
-                             BranchProbabilityInfo *BPI = nullptr);
+bool removeUnreachableBlocks(Function &F, LazyValueInfo *LVI = nullptr);
 
 /// Combine the metadata of two instructions so that K can replace J
 ///

llvm/lib/Analysis/BranchProbabilityInfo.cpp

@@ -639,14 +639,6 @@ BranchProbabilityInfo::printEdgeProbability(raw_ostream &OS,
   return OS;
 }
 
-void BranchProbabilityInfo::eraseBlock(const BasicBlock *BB) {
-  for (auto I = Probs.begin(), E = Probs.end(); I != E; ++I) {
-    auto Key = I->first;
-    if (Key.first == BB)
-      Probs.erase(Key);
-  }
-}
-
 void BranchProbabilityInfo::calculate(const Function &F, const LoopInfo &LI) {
   DEBUG(dbgs() << "---- Branch Probability Info : " << F.getName()
                << " ----\n\n");

llvm/lib/Transforms/Scalar/JumpThreading.cpp

@@ -182,7 +182,7 @@ bool JumpThreadingPass::runImpl(Function &F, TargetLibraryInfo *TLI_,
   // back edges. This works for normal cases but not for unreachable blocks as
   // they may have cycle with no back edge.
   bool EverChanged = false;
-  EverChanged |= removeUnreachableBlocks(F, LVI, BPI.get());
+  EverChanged |= removeUnreachableBlocks(F, LVI);
 
   FindLoopHeaders(F);
 
@@ -204,7 +204,7 @@ bool JumpThreadingPass::runImpl(Function &F, TargetLibraryInfo *TLI_,
         DEBUG(dbgs() << "  JT: Deleting dead block '" << BB->getName()
               << "' with terminator: " << *BB->getTerminator() << '\n');
         LoopHeaders.erase(BB);
-        dropBlockAnalysisResults(BB);
+        LVI->eraseBlock(BB);
         DeleteDeadBlock(BB);
         Changed = true;
         continue;
@@ -232,7 +232,7 @@ bool JumpThreadingPass::runImpl(Function &F, TargetLibraryInfo *TLI_,
         // for a block even if it doesn't get erased.  This isn't totally
         // awesome, but it allows us to use AssertingVH to prevent nasty
         // dangling pointer issues within LazyValueInfo.
-        dropBlockAnalysisResults(BB);
+        LVI->eraseBlock(BB);
         if (TryToSimplifyUncondBranchFromEmptyBlock(BB)) {
           Changed = true;
           // If we deleted BB and BB was the header of a loop, then the
@@ -715,7 +715,7 @@ bool JumpThreadingPass::ProcessBlock(BasicBlock *BB) {
       if (LoopHeaders.erase(SinglePred))
         LoopHeaders.insert(BB);
 
-      dropBlockAnalysisResults(SinglePred);
+      LVI->eraseBlock(SinglePred);
       MergeBasicBlockIntoOnlyPred(BB);
 
       return true;
@@ -1949,11 +1949,3 @@ bool JumpThreadingPass::TryToUnfoldSelectInCurrBB(BasicBlock *BB) {
   
   return false;
 }
-
-/// dropBlockAnalysisResults - Inform relevant analyzes that BB is going to
-/// be removed. This is important in order to prevent dangling pointer problems.
-void JumpThreadingPass::dropBlockAnalysisResults(BasicBlock *BB) {
-  LVI->eraseBlock(BB);
-  if (BPI)
-    BPI->eraseBlock(BB);
-}

llvm/lib/Transforms/Utils/Local.cpp

@@ -20,7 +20,6 @@
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/Statistic.h"
-#include "llvm/Analysis/BranchProbabilityInfo.h"
 #include "llvm/Analysis/EHPersonalities.h"
 #include "llvm/Analysis/InstructionSimplify.h"
 #include "llvm/Analysis/MemoryBuiltins.h"
@@ -1487,8 +1486,7 @@ void llvm::removeUnwindEdge(BasicBlock *BB) {
 /// removeUnreachableBlocksFromFn - Remove blocks that are not reachable, even
 /// if they are in a dead cycle.  Return true if a change was made, false
 /// otherwise.
-bool llvm::removeUnreachableBlocks(Function &F, LazyValueInfo *LVI, 
-                                   BranchProbabilityInfo *BPI) {
+bool llvm::removeUnreachableBlocks(Function &F, LazyValueInfo *LVI) {
   SmallPtrSet<BasicBlock*, 16> Reachable;
   bool Changed = markAliveBlocks(F, Reachable);
 
@@ -1511,8 +1509,6 @@ bool llvm::removeUnreachableBlocks(Function &F, LazyValueInfo *LVI,
         (*SI)->removePredecessor(&*BB);
     if (LVI)
       LVI->eraseBlock(&*BB);
-    if (BPI)
-      BPI->eraseBlock(&*BB);
     BB->dropAllReferences();
   }
 

llvm/test/Transforms/JumpThreading/dangling-pointers-in-bpi.ll

@@ -1,19 +0,0 @@
-; RUN: opt -S -jump-threading %s 2>&1 | FileCheck %s
-
-; Test that after removing basic block we had also removed it from BPI. If we
-; didn't there will be dangling pointer inside BPI. It can lead to a
-; miscalculation in the edge weight and possibly other problems.
-; Checks that we are not failing assertion inside AssettingVH
-
-; CHECK-NOT: An asserting value handle still pointed to this value
-
-define void @foo(i32 %n, i1 %cond) !prof !0 {
-; Record this block into BPI cache
-single-pred:
-  br i1 %cond, label %entry, label %entry, !prof !{!"branch_weights", i32 1, i32 1}
-
-entry:
-  ret void
-}
-
-!0 = !{!"function_entry_count", i64 1}

llvm/unittests/Analysis/BlockFrequencyInfoTest.cpp

@@ -54,11 +54,6 @@ protected:
     SMDiagnostic Err;
     return parseAssemblyString(ModuleStrig, Err, C);
   }
-
-  void destroyBFI() {
-    // Prevent AssertingVH from failing.
-    BPI->releaseMemory();
-  }
 };
 
 TEST_F(BlockFrequencyInfoTest, Basic) {
@@ -85,8 +80,6 @@ TEST_F(BlockFrequencyInfoTest, Basic) {
   EXPECT_EQ(BFI.getBlockProfileCount(BB3).getValue(), UINT64_C(100));
   EXPECT_EQ(BFI.getBlockProfileCount(BB1).getValue(), 100 * BB1Freq / BB0Freq);
   EXPECT_EQ(BFI.getBlockProfileCount(BB2).getValue(), 100 * BB2Freq / BB0Freq);
-
-  destroyBFI();
 }
 
 } // end anonymous namespace