[PATCH] D52575: [SimplifyCFG] Pass AggressiveInsts to DominatesMergePoint by reference. Remove null check.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:353
   // If we have seen this instruction before, don't count it again.
-  if (AggressiveInsts->count(I))
+  if (AggressiveInsts.count(I))
     return true;
----------------
xbolva00 wrote:
> craig.topper wrote:
> > xbolva00 wrote:
> > > craig.topper wrote:
> > > > xbolva00 wrote:
> > > > > AggressiveInsts.find(I) != AggressiveInsts.end()
> > > > Why?
> > > Count() counts them all, useless iterations after first occurance..
> > > 
> > > This condition just checks if there is any occurence of I in the AgressiveInst, so “find” is better choice I think
> > It's a set, it can't have more than one. The implementation of count knows this and in fact it is implemented as just find() != end().  std::map, std::set, SmallPtrSet all have good implentations of count. Only things like std::multiset/multimap would have an implementation that really has to count.
> Ok, sorry then, I didn't check the declaration :)
FWIW, C++20 adds contains() to all of the associative containers which provides the equivalent of find() != end() for multiset/multimap. And makes set/map/unordered_set/unordered_map read better than using count().


https://reviews.llvm.org/D52575