[PATCH] D20957: [JumpThreading] Prevent dangling pointer problems in BranchProbabilityInfo

[Igor Laevsky via llvm-commits]

igor-laevsky added inline comments.

================
Comment at: lib/Analysis/BranchProbabilityInfo.cpp:643
@@ +642,3 @@
+void BranchProbabilityInfo::eraseBlock(const BasicBlock *BB) {
+  for (auto I = Probs.begin(), E = Probs.end(); I != E; ++I) {
+    auto Key = I->first;
----------------
sanjoy wrote:
> Why do you need to do a linear search here?  Isn't `Probs` a `DenseMap<Edge, BranchProbability>`?  IOW, why not:
> 
> ```
> for (int i = 0; i < BB->numSuccessors(); i++)
>   Probs.erase({BB, i});
> ```
> 
Problem with this approach is that contents of the BB could have been changed since it was first recorded. If conditional terminator was replaced with unconditional jump, we may leave some BB references hanging in the map.

================
Comment at: unittests/Analysis/BlockFrequencyInfoTest.cpp:58
@@ +57,3 @@
+
+  void releaseBFI() {
+    // Prevent AssetingVH from failing
----------------
sanjoy wrote:
> Should this be called `releaseBPI`?
This is to be symmetric with buildBFI. Besides I suspect that BFI has the same issue and we will need to add "BFI->releaseMemory()" here.


http://reviews.llvm.org/D20957