<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 4, 2014 at 1:47 PM, Lang Hames <span dir="ltr"><<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Dave,<div><br></div><div>Echoing Arnaud's thoughts here - I've found that regression tests for heuristic register allocation optimizations such as this one aren't worth it: the signal to noise ratio on them is extremely low.</div></div></blockquote><div><br></div><div>Given the lack of perf buildbots watching this, it seems pretty easy to accidentally regress a bunch of this, which would be unfortunate.<br><br>Meh.<br><br>- David</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><br></div><div>Cheers,</div><div>Lang.</div><div><br></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 4, 2014 at 1:34 PM, Arnaud A. de Grandmaison <span dir="ltr"><<a href="mailto:arnaud.degrandmaison@arm.com" target="_blank">arnaud.degrandmaison@arm.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-GB" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi David,<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">The reason why there is no test did not make it into the commit log, but was in the pre-commit review request.<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">The reason is it is too complicated to come-up with a meaningful test because the tweaking address some global relationship amongst registers.<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">The tweaking done here was based on experiments with various benchmarks (eembc, spec2000, spec20006, …) and provided a several percent improvement (geomean) across those benchmarks.<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Cheers,<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Arnaud<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><div style="border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt"><div><div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0cm 0cm 0cm"><p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> David Blaikie [mailto:<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>] <br><b>Sent:</b> 04 November 2014 22:18<br><b>To:</b> Arnaud De Grandmaison<br><b>Cc:</b> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><b>Subject:</b> Re: [llvm] r221292 - [PBQP] Tweak spill costs and coalescing benefits<u></u><u></u></span></p></div></div><div><div><p class="MsoNormal"><u></u> <u></u></p><div><p class="MsoNormal"><u></u> <u></u></p><div><p class="MsoNormal"><u></u> <u></u></p><div><p class="MsoNormal">On Tue, Nov 4, 2014 at 12:51 PM, Arnaud A. de Grandmaison <<a href="mailto:arnaud.degrandmaison@arm.com" target="_blank">arnaud.degrandmaison@arm.com</a>> wrote:<u></u><u></u></p><p class="MsoNormal">Author: aadg<br>Date: Tue Nov 4 14:51:24 2014<br>New Revision: 221292<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=221292&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=221292&view=rev</a><br>Log:<br>[PBQP] Tweak spill costs and coalescing benefits<u></u><u></u></p><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">Is this intended to change the observable behavior of the allocator (coalescing more or less often, etc?) - a test would be nice for that.<br><br>If it isn't, it's helpful (well, to me, at least) to describe why something isn't tested in the commit message (just refactoring to lay the foundation for a future change, etc).<u></u><u></u></p></div><div><p class="MsoNormal"> <u></u><u></u></p></div><blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm"><p class="MsoNormal"><br>This patch improves how the different costs (register, interference, spill<br>and coalescing) relates together. The assumption is now that:<br> - coalescing (or any other "side effect" of reg alloc) is negative, and<br> instead of being derived from a spill cost, they use the block<br> frequency info.<br> - spill costs are in the [MinSpillCost:+inf( range<br> - register or interference costs are in [0.0:MinSpillCost( or +inf<br><br>The current MinSpillCost is set to 10.0, which is a random value high<br>enough that the current constraint builders do not need to worry about<br>when settings costs. It would however be worth adding a normalization<br>step for register and interference costs as the last step in the<br>constraint builder chain to ensure they are not greater than SpillMinCost<br>(unless this has some sense for some architectures). This would work well<br>with the current builder pipeline, where all costs are tweaked relatively<br>to each others, but could grow above MinSpillCost if the pipeline is<br>deep enough.<br><br>The current heuristic is tuned to depend rather on the number of uses of<br>a live interval rather than a density of uses, as used by the greedy<br>allocator. This heuristic provides a few percent improvement on a number<br>of benchmarks (eembc, spec, ...) and will definitely need to change once<br>spill placement is implemented: the current spill placement is really<br>ineficient, so making the cost proportionnal to the number of use is a<br>clear win.<br><br>Modified:<br> llvm/trunk/include/llvm/CodeGen/CalcSpillWeights.h<br> llvm/trunk/lib/CodeGen/CalcSpillWeights.cpp<br> llvm/trunk/lib/CodeGen/RegAllocGreedy.cpp<br> llvm/trunk/lib/CodeGen/RegAllocPBQP.cpp<br><br>Modified: llvm/trunk/include/llvm/CodeGen/CalcSpillWeights.h<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/CalcSpillWeights.h?rev=221292&r1=221291&r2=221292&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/CalcSpillWeights.h?rev=221292&r1=221291&r2=221292&view=diff</a><br>==============================================================================<br>--- llvm/trunk/include/llvm/CodeGen/CalcSpillWeights.h (original)<br>+++ llvm/trunk/include/llvm/CodeGen/CalcSpillWeights.h Tue Nov 4 14:51:24 2014<br>@@ -30,8 +30,10 @@ namespace llvm {<br> /// @param UseDefFreq Expected number of executed use and def instructions<br> /// per function call. Derived from block frequencies.<br> /// @param Size Size of live interval as returnexd by getSize()<br>+ /// @param NumInstr Number of instructions using this live interval<br> ///<br>- static inline float normalizeSpillWeight(float UseDefFreq, unsigned Size) {<br>+ static inline float normalizeSpillWeight(float UseDefFreq, unsigned Size,<br>+ unsigned NumInstr) {<br> // The constant 25 instructions is added to avoid depending too much on<br> // accidental SlotIndex gaps for small intervals. The effect is that small<br> // intervals have a spill weight that is mostly proportional to the number<br>@@ -44,7 +46,7 @@ namespace llvm {<br> /// spill weight and allocation hint.<br> class VirtRegAuxInfo {<br> public:<br>- typedef float (*NormalizingFn)(float, unsigned);<br>+ typedef float (*NormalizingFn)(float, unsigned, unsigned);<br><br> private:<br> MachineFunction &MF;<br><br>Modified: llvm/trunk/lib/CodeGen/CalcSpillWeights.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CalcSpillWeights.cpp?rev=221292&r1=221291&r2=221292&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CalcSpillWeights.cpp?rev=221292&r1=221291&r2=221292&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/CodeGen/CalcSpillWeights.cpp (original)<br>+++ llvm/trunk/lib/CodeGen/CalcSpillWeights.cpp Tue Nov 4 14:51:24 2014<br>@@ -100,6 +100,7 @@ VirtRegAuxInfo::calculateSpillWeightAndH<br> MachineLoop *loop = nullptr;<br> bool isExiting = false;<br> float totalWeight = 0;<br>+ unsigned numInstr = 0; // Number of instructions using li<br> SmallPtrSet<MachineInstr*, 8> visited;<br><br> // Find the best physreg hint and the best virtreg hint.<br>@@ -116,6 +117,7 @@ VirtRegAuxInfo::calculateSpillWeightAndH<br> I = mri.reg_instr_begin(li.reg), E = mri.reg_instr_end();<br> I != E; ) {<br> MachineInstr *mi = &*(I++);<br>+ numInstr++;<br> if (mi->isIdentityCopy() || mi->isImplicitDef() || mi->isDebugValue())<br> continue;<br> if (!visited.insert(mi))<br>@@ -189,5 +191,5 @@ VirtRegAuxInfo::calculateSpillWeightAndH<br> if (isRematerializable(li, LIS, *MF.getSubtarget().getInstrInfo()))<br> totalWeight *= 0.5F;<br><br>- li.weight = normalize(totalWeight, li.getSize());<br>+ li.weight = normalize(totalWeight, li.getSize(), numInstr);<br> }<br><br>Modified: llvm/trunk/lib/CodeGen/RegAllocGreedy.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegAllocGreedy.cpp?rev=221292&r1=221291&r2=221292&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegAllocGreedy.cpp?rev=221292&r1=221291&r2=221292&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/CodeGen/RegAllocGreedy.cpp (original)<br>+++ llvm/trunk/lib/CodeGen/RegAllocGreedy.cpp Tue Nov 4 14:51:24 2014<br>@@ -1789,9 +1789,11 @@ unsigned RAGreedy::tryLocalSplit(LiveInt<br> // instructions.<br> //<br> // Try to guess the size of the new interval.<br>- const float EstWeight = normalizeSpillWeight(blockFreq * (NewGaps + 1),<br>- Uses[SplitBefore].distance(Uses[SplitAfter]) +<br>- (LiveBefore + LiveAfter)*SlotIndex::InstrDist);<br>+ const float EstWeight = normalizeSpillWeight(<br>+ blockFreq * (NewGaps + 1),<br>+ Uses[SplitBefore].distance(Uses[SplitAfter]) +<br>+ (LiveBefore + LiveAfter) * SlotIndex::InstrDist,<br>+ 1);<br> // Would this split be possible to allocate?<br> // Never allocate all gaps, we wouldn't be making progress.<br> DEBUG(dbgs() << " w=" << EstWeight);<br><br>Modified: llvm/trunk/lib/CodeGen/RegAllocPBQP.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegAllocPBQP.cpp?rev=221292&r1=221291&r2=221292&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegAllocPBQP.cpp?rev=221292&r1=221291&r2=221292&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/CodeGen/RegAllocPBQP.cpp (original)<br>+++ llvm/trunk/lib/CodeGen/RegAllocPBQP.cpp Tue Nov 4 14:51:24 2014<br>@@ -150,11 +150,17 @@ public:<br> void apply(PBQPRAGraph &G) override {<br> LiveIntervals &LIS = G.getMetadata().LIS;<br><br>+ // A minimum spill costs, so that register constraints can can be set<br>+ // without normalization in the [0.0:MinSpillCost( interval.<br>+ const PBQP::PBQPNum MinSpillCost = 10.0;<br>+<br> for (auto NId : G.nodeIds()) {<br> PBQP::PBQPNum SpillCost =<br> LIS.getInterval(G.getNodeMetadata(NId).getVReg()).weight;<br> if (SpillCost == 0.0)<br> SpillCost = std::numeric_limits<PBQP::PBQPNum>::min();<br>+ else<br>+ SpillCost += MinSpillCost;<br> PBQPRAGraph::RawVector NodeCosts(G.getNodeCosts(NId));<br> NodeCosts[PBQP::RegAlloc::getSpillOptionIdx()] = SpillCost;<br> G.setNodeCosts(NId, std::move(NodeCosts));<br>@@ -350,11 +356,8 @@ public:<br> unsigned DstReg = CP.getDstReg();<br> unsigned SrcReg = CP.getSrcReg();<br><br>- const float CopyFactor = 0.5; // Cost of copy relative to load. Current<br>- // value plucked randomly out of the air.<br>-<br>- PBQP::PBQPNum CBenefit =<br>- CopyFactor * LiveIntervals::getSpillWeight(false, true, &MBFI, &MI);<br>+ const float Scale = 1.0f / MBFI.getEntryFreq();<br>+ PBQP::PBQPNum CBenefit = MBFI.getBlockFreq(&MBB).getFrequency() * Scale;<br><br> if (CP.isPhys()) {<br> if (!MF.getRegInfo().isAllocatable(DstReg))<br>@@ -607,12 +610,20 @@ void RegAllocPBQP::finalizeAlloc(Machine<br> }<br> }<br><br>+static inline float normalizePBQPSpillWeight(float UseDefFreq, unsigned Size,<br>+ unsigned NumInstr) {<br>+ // All intervals have a spill weight that is mostly proportional to the number<br>+ // of uses, with uses in loops having a bigger weight.<br>+ return NumInstr * normalizeSpillWeight(UseDefFreq, Size, 1);<br>+}<br>+<br> bool RegAllocPBQP::runOnMachineFunction(MachineFunction &MF) {<br> LiveIntervals &LIS = getAnalysis<LiveIntervals>();<br> MachineBlockFrequencyInfo &MBFI =<br> getAnalysis<MachineBlockFrequencyInfo>();<br><br>- calculateSpillWeightsAndHints(LIS, MF, getAnalysis<MachineLoopInfo>(), MBFI);<br>+ calculateSpillWeightsAndHints(LIS, MF, getAnalysis<MachineLoopInfo>(), MBFI,<br>+ normalizePBQPSpillWeight);<br><br> VirtRegMap &VRM = getAnalysis<VirtRegMap>();<br><br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><u></u><u></u></p></blockquote></div><p class="MsoNormal"><u></u> <u></u></p></div></div></div></div></div></div></div><br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>