[PATCH] D48790: Update MemorySSA in Local utils removing blocks.

[Chijun Sima via Phabricator via llvm-commits]

NutshellySima added a comment.

Thanks for the improvement! Comments are inline.



================
Comment at: lib/Transforms/Utils/Local.cpp:2231
 
+  // FIXME: May reuse this set below if deletion order does not matter.
+  SmallPtrSet<BasicBlock *, 16> DeadBlockSet;
----------------
asbirlea wrote:
> NutshellySima wrote:
> > Yes, I think the deletion order doesn't matter.
> It seems to make sense to use this in the first iteration over the blocks in F that follows, but not the second one doing the erase, since that's better to be iterator based.
> Also search DeadBlockSet instead of Reachable, assuming more blocks are live than dead.
I think maybe you can replace `ToDeleteBBs` with `DeadBlockSet`.
For assuming there are 
> more blocks are live than dead
, if you are interested, you can run CTMarks/have some other benchmarks to see whether there is performance degradation.


Repository:
  rL LLVM

https://reviews.llvm.org/D48790