<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Oct 16, 2014 at 8:33 AM, Chad Rosier <span dir="ltr"><<a href="mailto:mcrosier@codeaurora.org" target="_blank">mcrosier@codeaurora.org</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 Sean,<br>
Thanks for your input.  Comments below.<br>
<span class=""><br>
>>! In D5786#4, @silvas wrote:<br>
> The ideal solution to "given I have branch weight metadata giving me the relative probabilities of each case label, in what order should I do two-way comparisons to minimize the expected number of comparisons needed to reach the cases at run-time" is not a balanced binary tree; it is a Huffman tree (where the case probabilities are what is usually called "symbol probabilities" when talking about Huffman trees.)<br>
<br>
</span>To clarify, I didn't mean "balanced" in the traditional sense, but balanced based on probabilities.  I do recall our conversation (+Chandler +Has) on IRC about Huffman trees.<br>
<span class=""><br>
> I don't think this patch is the right long-term approach.<br>
<br>
</span>I believe I said that.. LLVM prefers incremental; I believe this is a reasonable first step.  I understand the Huffman tree is an entirely different implementation, but the proposed solution is on the order of 50 lines of real code, straight-forward, and should require little or no maintenance.  It *should* also provide a fairly good ROI, but I understand that that is yet to be proven.<br></blockquote><div><br></div><div>Cool. No rush.<br></div><div><br></div><div>-- Sean Silva</div><div> </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">
<span class=""><br>
>I do think this patch is useful for identifying the possible performance win of using branch weight info to guide switch lowering. If the current patch doesn't result in a measurable speedup across a variety of benchmarks, it is unlikely to be worth much effort in improving our lowering by using branch weight metadata. You mentioned you ran spec2000/spec2006; what were the performance benefits?<br>
<br>
</span>I mentioned that I was working on getting numbers, but I've now been side tracked due to an internal release and the upcoming Dev Meeting.  I don't know when I'll have the bandwidth to generate the numbers, so I suggested other give it a try if they so wish.  Feel free to do so yourself.<br>
<span class=""><br>
> If you aren't familiar with Huffman trees, they are basically based on a couple simple observations (imagine you have a binary tree, where each branching point represents a test, and the leaves represent case labels):<br>
><br>
> - the tree is full (i.e. all non-leaf nodes have two children). This means that if a case label is one of the "deepest" labels (longest path from the root), then its sibling must be as well.<br>
> - if case A has a greater probability than case B, then the leaf containing case A must be closer to the root (or the same distance) as case B.<br>
> - there is an optimal tree where the two least likely cases are siblings.<br>
><br>
> That's it. The algorithm is then to start with all case labels in a min-heap by probability, and to repeatedly pop off the two least likely cases, merge them into a fragment of the final tree, which notionally has a probability equal to the sum of their probabilities, and push this merged tree fragment back on the heap. Eventually, the heap contains only one element which is your final tree.<br>
><br>
> Honestly, having branch probabilities is really a game-changer for switch lowering and probably merits a different approach (or reconsidering the current approach). You should probably take a look (if you haven't already) at the paper "A Superoptimizer Analysis of Multiway Branch Code Generation" in <a href="https://gcc.gnu.org/wiki/HomePage?action=AttachFile&do=get&target=gcc-2008-proceedings.pdf" target="_blank">https://gcc.gnu.org/wiki/HomePage?action=AttachFile&do=get&target=gcc-2008-proceedings.pdf</a><br>
<br>
</span>I'm aware of the paper.  In fact, this approach is based on results in the 2006 Summit in the paper titled, "Switch Statement Case Reordering FDO."  That works shows that hoisting the hottest two cases in spec2000/perlbmk improved performance by 17%.  I don't expect to get nearly that performance, but I do expect to see some gain.<br>
<br>
I understand your concern, but I don't agree that this patch is a step in the wrong direction (or should be rejected for the ideal).  If the patch were more pervasive, then I would proceed with caution.  However, it is not and it should have a fair ROI.  If, and when, someone comes along to implement the Huffman approach it will be trivial to revert this change.<br>
<br>
  Chad<br>
<br>
<a href="http://reviews.llvm.org/D5786" target="_blank">http://reviews.llvm.org/D5786</a><br>
<br>
<br>
</blockquote></div><br></div></div>