[PATCH] D37940: Peel off the dominant case in switch statement

[Rong Xu via llvm-commits]

On Fri, Sep 15, 2017 at 4:25 PM, David Li via Phabricator <
reviews at reviews.llvm.org> wrote:

> davidxl added inline comments.
>
>
> ================
> Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:5680
>
> +  if (PeelDominantCase(SI, Builder))
> +    return SimplifyCFG(BB, TTI, BonusInstThreshold, AC) | true;
> ----------------
> hans wrote:
> > I don't think we should do this if SwitchToLookupTable (the call below)
> can turn the swich into a lookup instead.
> >
> > In fact, I don't think this should be done at the IR level at all, since
> it's more of a switch lowering issue. SelectionDAGBuilder::visitSwitch()
> would be a better place.
> >
> > That code already takes case weights into account, and when lowering to
> a binary search tree, it will balance it based on weight, favoring cases
> that are hot. Do you find that that's not sufficient? (I'm willing to
> believe that's the case, but I'd like to see it argued.)
> Peeling the  dominating case avoids a memory read (table lookup). Besides
> it trades a indirect branch for a a highly biased direct branch (which is
> usually highly predictable by the branch predictor).
>
> Like looping which happens at higher level,  switch peeling can also
> enable more possible surrounding optimizations (eg like jump threading).
> Peeling it out also probably also makes it easier to for better code layout.
>
> On the other hand, this should not be done when size optimization is on.
>
Yes. I agree.  Will update the patch with this.

>
>
>
>
>
> https://reviews.llvm.org/D37940
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170915/99a506d0/attachment.html>