[llvm] r230769 - Switch a std::map to a DenseMap in CodeGenRegisters.

[Nick Lewycky]

On 27 February 2015 at 09:57, Owen Anderson <resistor at mac.com> wrote:

> Author: resistor
> Date: Fri Feb 27 11:57:01 2015
> New Revision: 230769
>
> URL: http://llvm.org/viewvc/llvm-project?rev=230769&view=rev
> Log:
> Switch a std::map to a DenseMap in CodeGenRegisters.
>
> The keys of the map are unique by pointer address, so there's no need
> to use the llvm::less comparator. This allows us to use DenseMap
> instead, which reduces tblgen time by 20% on my stress test.
>
> Modified:
>     llvm/trunk/utils/TableGen/CodeGenRegisters.cpp
>     llvm/trunk/utils/TableGen/CodeGenRegisters.h
>     llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp
>
> Modified: llvm/trunk/utils/TableGen/CodeGenRegisters.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenRegisters.cpp?rev=230769&r1=230768&r2=230769&view=diff
>
> ==============================================================================
> --- llvm/trunk/utils/TableGen/CodeGenRegisters.cpp (original)
> +++ llvm/trunk/utils/TableGen/CodeGenRegisters.cpp Fri Feb 27 11:57:01 2015
> @@ -258,6 +258,8 @@ CodeGenRegister::computeSubRegs(CodeGenR
>
>      // Look at the possible compositions of Idx.
>      // They may not all be supported by SR.
> +    // NOTE: Iteration order does not matter here because the EnumValue's
> +    // of subreg indices are unique.
>

I think this introduced non-determinism. This iteration order changes the
order of which items get evaluted to be put into Indices which is a vector,
and that feeds into other things. The net result is that one build of clang
has these symbols:

00000000012b0a50 t
_ZN4llvmL51QQQQPR_with_dsub_2_then_ssub_0GetRawAllocationOrderERKNS_15MachineFunctionE
00000000012b0a60 t
_ZN4llvmL51QQQQPR_with_dsub_5_then_ssub_0GetRawAllocationOrderERKNS_15MachineFunctionE
00000000012b0a70 t
_ZN4llvmL51QQQQPR_with_dsub_7_then_ssub_0GetRawAllocationOrderERKNS_15MachineFunctionE

while the other has these:

00000000012b0a50 t
_ZN4llvmL51QQQQPR_with_dsub_2_then_ssub_0GetRawAllocationOrderERKNS_15MachineFunctionE
00000000012b0a60 t
_ZN4llvmL51QQQQPR_with_dsub_4_then_ssub_0GetRawAllocationOrderERKNS_15MachineFunctionE
00000000012b0a70 t
_ZN4llvmL51QQQQPR_with_dsub_6_then_ssub_0GetRawAllocationOrderERKNS_15MachineFunctionE

That difference is a bootstrap miscompare. Besides the symbol names, every
instruction is identical.

Let me know if you'd like me help debugging further. I'm going to partially
revert only this comment and the typedef change in CodeGenRegisters.h,
since the rest is code cleanup.

Nick

     for (CodeGenSubRegIndex::CompMap::const_iterator I = Comps.begin(),
>             E = Comps.end(); I != E; ++I) {
>        SubRegMap::const_iterator SRI = Map.find(I->first);
>
> Modified: llvm/trunk/utils/TableGen/CodeGenRegisters.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenRegisters.h?rev=230769&r1=230768&r2=230769&view=diff
>
> ==============================================================================
> --- llvm/trunk/utils/TableGen/CodeGenRegisters.h (original)
> +++ llvm/trunk/utils/TableGen/CodeGenRegisters.h Fri Feb 27 11:57:01 2015
> @@ -74,8 +74,7 @@ namespace llvm {
>      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.
>
> Modified: llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp?rev=230769&r1=230768&r2=230769&view=diff
>
> ==============================================================================
> --- llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp (original)
> +++ llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp Fri Feb 27 11:57:01
> 2015
> @@ -610,17 +610,19 @@ static void printMask(raw_ostream &OS, u
>  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;
>  }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150302/fa88bf27/attachment.html>