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

Rong Xu via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 18 13:05:10 PDT 2017


On Mon, Sep 18, 2017 at 12:14 PM, Hans Wennborg via Phabricator <
reviews at reviews.llvm.org> wrote:

> hans added inline comments.
>
>
> ================
> Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:5680
>
> +  if (PeelDominantCase(SI, Builder))
> +    return SimplifyCFG(BB, TTI, BonusInstThreshold, AC) | true;
> ----------------
> davidxl wrote:
> > 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.
> >
> >
> >
> SwitchToLookupTable creates lookup tables for switches that are used to
> select from a set of constant values (this is different from jump tables,
> sorry the names here are confusing), so there is no indirect branch. I
> think peeling off the common case is probably not a good idea for those
> kinds of switches.
>
> I can see that peeling early might enable other high-level optimizations.
> But that's true in some sense for switch lowering in general. I'd suggest
> to at least look into how peeling in the regular switch lowering code at
> SelectionDAGBuilder::visitSwitch() would look like. It seems to me like
> the natural place to do it, and I suspect it's easier to do there.
>
> Thanks for the suggestion. Doing this late does avoid some interactions
with the folding in simplifyCFG. I'll implement this in lowering phrase and
compare the performance.

-rong



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


More information about the llvm-commits mailing list