[llvm] r245752 - Range-for-ify some things in GlobalMerge

[Charlie Turner via llvm-commits]

Hi David, I think there's a mistake here:

You've replaced

-  if (EnableGlobalMergeOnConst)
-    for (DenseMap<unsigned, SmallVector<GlobalVariable*, 16> >::iterator
-         I = ConstGlobals.begin(), E = ConstGlobals.end(); I != E; ++I)
-      if (I->second.size() > 1)
-        Changed |= doMerge(I->second, M, true, I->first);

with

+  for (auto &P : ConstGlobals)
+    if (P.second.size() > 1)
+      Changed |= doMerge(P.second, M, true, P.first);

Looks like you missed the if (EnableGlobalMergeOnConst) guard. I'm
seeing a 3.69% regression in spec2000's 253.perlbmk with that guard
removed.

Could you take a look please?

Thanks for your time,
Charlie.

On 21 August 2015 at 23:19, David Blaikie via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: dblaikie
> Date: Fri Aug 21 17:19:06 2015
> New Revision: 245752
>
> URL: http://llvm.org/viewvc/llvm-project?rev=245752&view=rev
> Log:
> Range-for-ify some things in GlobalMerge
>
> Modified:
>     llvm/trunk/lib/CodeGen/GlobalMerge.cpp
>
> Modified: llvm/trunk/lib/CodeGen/GlobalMerge.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalMerge.cpp?rev=245752&r1=245751&r2=245752&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/GlobalMerge.cpp (original)
> +++ llvm/trunk/lib/CodeGen/GlobalMerge.cpp Fri Aug 21 17:19:06 2015
> @@ -135,7 +135,7 @@ namespace {
>                   Module &M, bool isConst, unsigned AddrSpace) const;
>      /// \brief Merge everything in \p Globals for which the corresponding bit
>      /// in \p GlobalSet is set.
> -    bool doMerge(SmallVectorImpl<GlobalVariable *> &Globals,
> +    bool doMerge(const SmallVectorImpl<GlobalVariable *> &Globals,
>                   const BitVector &GlobalSet, Module &M, bool isConst,
>                   unsigned AddrSpace) const;
>
> @@ -406,15 +406,14 @@ bool GlobalMerge::doMerge(SmallVectorImp
>    return Changed;
>  }
>
> -bool GlobalMerge::doMerge(SmallVectorImpl<GlobalVariable *> &Globals,
> +bool GlobalMerge::doMerge(const SmallVectorImpl<GlobalVariable *> &Globals,
>                            const BitVector &GlobalSet, Module &M, bool isConst,
>                            unsigned AddrSpace) const {
> +  assert(Globals.size() > 1);
>
>    Type *Int32Ty = Type::getInt32Ty(M.getContext());
>    auto &DL = M.getDataLayout();
>
> -  assert(Globals.size() > 1);
> -
>    DEBUG(dbgs() << " Trying to merge set, starts with #"
>                 << GlobalSet.find_first() << "\n");
>
> @@ -562,21 +561,17 @@ bool GlobalMerge::doInitialization(Modul
>      }
>    }
>
> -  for (DenseMap<unsigned, SmallVector<GlobalVariable*, 16> >::iterator
> -       I = Globals.begin(), E = Globals.end(); I != E; ++I)
> -    if (I->second.size() > 1)
> -      Changed |= doMerge(I->second, M, false, I->first);
> -
> -  for (DenseMap<unsigned, SmallVector<GlobalVariable*, 16> >::iterator
> -       I = BSSGlobals.begin(), E = BSSGlobals.end(); I != E; ++I)
> -    if (I->second.size() > 1)
> -      Changed |= doMerge(I->second, M, false, I->first);
> -
> -  if (EnableGlobalMergeOnConst)
> -    for (DenseMap<unsigned, SmallVector<GlobalVariable*, 16> >::iterator
> -         I = ConstGlobals.begin(), E = ConstGlobals.end(); I != E; ++I)
> -      if (I->second.size() > 1)
> -        Changed |= doMerge(I->second, M, true, I->first);
> +  for (auto &P : Globals)
> +    if (P.second.size() > 1)
> +      Changed |= doMerge(P.second, M, false, P.first);
> +
> +  for (auto &P : BSSGlobals)
> +    if (P.second.size() > 1)
> +      Changed |= doMerge(P.second, M, false, P.first);
> +
> +  for (auto &P : ConstGlobals)
> +    if (P.second.size() > 1)
> +      Changed |= doMerge(P.second, M, true, P.first);
>
>    return Changed;
>  }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits