[PATCH] Switch a std::map to a DenseMap in CodeGenRegisters

[Duncan P. N. Exon Smith]

> On 2015 Feb 17, at 09:47, Owen Anderson <resistor at mac.com> wrote:
> 
> Added checking of uniqueness to one loop that would potentially change iteration order here.  I'd prefer *not* to add checking to computeSubRegs() in CodeGenRegisters.cpp because the checking would be quite heavyweight, the method is very hot, and I am explicitly trying to improve LLVM build times in Release+Asserts configs here.
> 

Right, right :).  Two DenseMaps are probably still faster than a
std::map, but I get your point.  I might prefer a comment in
computeSubRegs() to annotate that the iteration order is
unimportant, but maybe it's obvious enough to those that work on
the code?

This LGTM after your explanations (thanks for the context).

> 
> REPOSITORY
>  rL LLVM
> 
> http://reviews.llvm.org/D7544
> 
> Files:
>  utils/TableGen/CodeGenRegisters.h
>  utils/TableGen/RegisterInfoEmitter.cpp
> 
> Index: utils/TableGen/CodeGenRegisters.h
> ===================================================================
> --- utils/TableGen/CodeGenRegisters.h
> +++ utils/TableGen/CodeGenRegisters.h
> @@ -74,8 +74,7 @@
>     std::string getQualifiedName() const;
> 
>     // Map of composite subreg indices.
> -    typedef std::map<CodeGenSubRegIndex *, CodeGenSubRegIndex *,
> -                     deref<llvm::less>> CompMap;
> +    typedef DenseMap<CodeGenSubRegIndex *, CodeGenSubRegIndex *> CompMap;
> 
>     // Returns the subreg index that results from composing this with Idx.
>     // Returns NULL if this and Idx don't compose.
> Index: utils/TableGen/RegisterInfoEmitter.cpp
> ===================================================================
> --- utils/TableGen/RegisterInfoEmitter.cpp
> +++ utils/TableGen/RegisterInfoEmitter.cpp
> @@ -610,17 +610,19 @@
> static bool combine(const CodeGenSubRegIndex *Idx,
>                     SmallVectorImpl<CodeGenSubRegIndex*> &Vec) {
>   const CodeGenSubRegIndex::CompMap &Map = Idx->getComposites();
> -  for (CodeGenSubRegIndex::CompMap::const_iterator
> -       I = Map.begin(), E = Map.end(); I != E; ++I) {
> -    CodeGenSubRegIndex *&Entry = Vec[I->first->EnumValue - 1];
> -    if (Entry && Entry != I->second)
> +  for (const auto &I : Map) {
> +    CodeGenSubRegIndex *&Entry = Vec[I.first->EnumValue - 1];
> +    if (Entry && Entry != I.second)
>       return false;
>   }
> 
>   // All entries are compatible. Make it so.
> -  for (CodeGenSubRegIndex::CompMap::const_iterator
> -       I = Map.begin(), E = Map.end(); I != E; ++I)
> -    Vec[I->first->EnumValue - 1] = I->second;
> +  for (const auto &I : Map) {
> +    auto *&Entry = Vec[I.first->EnumValue - 1];
> +    assert((!Entry || Entry == I.second) &&
> +           "Expected EnumValue to be unique");
> +    Entry = I.second;
> +  }
>   return true;
> }
> 
> EMAIL PREFERENCES
>  http://reviews.llvm.org/settings/panel/emailpreferences/
> <D7544.20095.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits