[PATCH] [PGO] Hoist hot case statement from switches

[Sean Silva]

On Thu, Oct 16, 2014 at 8:33 AM, Chad Rosier <mcrosier at codeaurora.org>
wrote:

> Hi Sean,
> Thanks for your input.  Comments below.
>
> >>! In D5786#4, @silvas wrote:
> > 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.)
>
> 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.
>
> > I don't think this patch is the right long-term approach.
>
> 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.
>

Cool. No rush.

-- Sean Silva


>
> >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?
>
> 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.
>
> > 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):
> >
> > - 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.
> > - 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.
> > - there is an optimal tree where the two least likely cases are siblings.
> >
> > 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.
> >
> > 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
> https://gcc.gnu.org/wiki/HomePage?action=AttachFile&do=get&target=gcc-2008-proceedings.pdf
>
> 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.
>
> 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.
>
>   Chad
>
> http://reviews.llvm.org/D5786
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141016/033e619f/attachment.html>