<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Aug 25, 2015 at 3:25 AM, Charlie Turner <span dir="ltr"><<a href="mailto:charlesturner7c5@gmail.com" target="_blank">charlesturner7c5@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi David, I think there's a mistake here:<br>
<br>
You've replaced<br>
<span class=""><br>
- if (EnableGlobalMergeOnConst)<br>
- for (DenseMap<unsigned, SmallVector<GlobalVariable*, 16> >::iterator<br>
- I = ConstGlobals.begin(), E = ConstGlobals.end(); I != E; ++I)<br>
- if (I->second.size() > 1)<br>
- Changed |= doMerge(I->second, M, true, I->first);<br>
<br>
</span>with<br>
<span class=""><br>
+ for (auto &P : ConstGlobals)<br>
+ if (P.second.size() > 1)<br>
+ Changed |= doMerge(P.second, M, true, P.first);<br>
<br>
</span>Looks like you missed the if (EnableGlobalMergeOnConst) guard. I'm<br>
seeing a 3.69% regression in spec2000's 253.perlbmk with that guard<br>
removed.<br>
<br>
Could you take a look please?<br></blockquote><div><br>Yep, looks like exactly that. Added the conditional back in <span style="color:rgb(0,0,0)">r245954.<br></span><br>Thanks!<br>- Dave<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
Thanks for your time,<br>
Charlie.<br>
<br>
On 21 August 2015 at 23:19, David Blaikie via llvm-commits<br>
<div class=""><div class="h5"><<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
> Author: dblaikie<br>
> Date: Fri Aug 21 17:19:06 2015<br>
> New Revision: 245752<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=245752&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=245752&view=rev</a><br>
> Log:<br>
> Range-for-ify some things in GlobalMerge<br>
><br>
> Modified:<br>
> llvm/trunk/lib/CodeGen/GlobalMerge.cpp<br>
><br>
> Modified: llvm/trunk/lib/CodeGen/GlobalMerge.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalMerge.cpp?rev=245752&r1=245751&r2=245752&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalMerge.cpp?rev=245752&r1=245751&r2=245752&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/CodeGen/GlobalMerge.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/GlobalMerge.cpp Fri Aug 21 17:19:06 2015<br>
> @@ -135,7 +135,7 @@ namespace {<br>
> Module &M, bool isConst, unsigned AddrSpace) const;<br>
> /// \brief Merge everything in \p Globals for which the corresponding bit<br>
> /// in \p GlobalSet is set.<br>
> - bool doMerge(SmallVectorImpl<GlobalVariable *> &Globals,<br>
> + bool doMerge(const SmallVectorImpl<GlobalVariable *> &Globals,<br>
> const BitVector &GlobalSet, Module &M, bool isConst,<br>
> unsigned AddrSpace) const;<br>
><br>
> @@ -406,15 +406,14 @@ bool GlobalMerge::doMerge(SmallVectorImp<br>
> return Changed;<br>
> }<br>
><br>
> -bool GlobalMerge::doMerge(SmallVectorImpl<GlobalVariable *> &Globals,<br>
> +bool GlobalMerge::doMerge(const SmallVectorImpl<GlobalVariable *> &Globals,<br>
> const BitVector &GlobalSet, Module &M, bool isConst,<br>
> unsigned AddrSpace) const {<br>
> + assert(Globals.size() > 1);<br>
><br>
> Type *Int32Ty = Type::getInt32Ty(M.getContext());<br>
> auto &DL = M.getDataLayout();<br>
><br>
> - assert(Globals.size() > 1);<br>
> -<br>
> DEBUG(dbgs() << " Trying to merge set, starts with #"<br>
> << GlobalSet.find_first() << "\n");<br>
><br>
> @@ -562,21 +561,17 @@ bool GlobalMerge::doInitialization(Modul<br>
> }<br>
> }<br>
><br>
> - for (DenseMap<unsigned, SmallVector<GlobalVariable*, 16> >::iterator<br>
> - I = Globals.begin(), E = Globals.end(); I != E; ++I)<br>
> - if (I->second.size() > 1)<br>
> - Changed |= doMerge(I->second, M, false, I->first);<br>
> -<br>
> - for (DenseMap<unsigned, SmallVector<GlobalVariable*, 16> >::iterator<br>
> - I = BSSGlobals.begin(), E = BSSGlobals.end(); I != E; ++I)<br>
> - if (I->second.size() > 1)<br>
> - Changed |= doMerge(I->second, M, false, I->first);<br>
> -<br>
> - if (EnableGlobalMergeOnConst)<br>
> - for (DenseMap<unsigned, SmallVector<GlobalVariable*, 16> >::iterator<br>
> - I = ConstGlobals.begin(), E = ConstGlobals.end(); I != E; ++I)<br>
> - if (I->second.size() > 1)<br>
> - Changed |= doMerge(I->second, M, true, I->first);<br>
> + for (auto &P : Globals)<br>
> + if (P.second.size() > 1)<br>
> + Changed |= doMerge(P.second, M, false, P.first);<br>
> +<br>
> + for (auto &P : BSSGlobals)<br>
> + if (P.second.size() > 1)<br>
> + Changed |= doMerge(P.second, M, false, P.first);<br>
> +<br>
> + for (auto &P : ConstGlobals)<br>
> + if (P.second.size() > 1)<br>
> + Changed |= doMerge(P.second, M, true, P.first);<br>
><br>
> return Changed;<br>
> }<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>