[PATCH] Improving LowerSwitch behavior for contiguous ranges

[Marcello Maggioni]

Hello,

thank you for your feedback guys!

Answers to everybody:

- Pete: Thanks for that, seems like clang-format-diff doesn't understand
DEBUG() macros ... :P
- Philip: I removed as much whitespace changes as I could find
- Eric: I changed the comment in head to switchConvert to explain the new
Upper and LowerBound arguments (is that what you meant?)

Cheers,
Marcello


2014-06-10 18:14 GMT+01:00 Eric Christopher <echristo at gmail.com>:

> While you're in there updating the documentation for switchConvert
> would be good as well.
>
> -eric
>
> On Tue, Jun 10, 2014 at 5:26 AM, Marcello Maggioni <hayarms at gmail.com>
> wrote:
> > Hello,
> >
> > when LowerSwitch transforms a switch instruction into a tree of ifs it is
> > actually performing a binary search into the various case ranges, to see
> if
> > the current value falls into one cases range of values.
> >
> > So, if we have a program with something like this:
> >
> > switch (a) {
> > case 0:
> >   do0();
> >   break;
> > case 1:
> >   do1();
> >   break;
> > case 2:
> >   do2();
> >   break;
> > default:
> >   break;
> > }
> >
> > the code produced is something like this:
> >
> >   if (a < 1) {
> >     if (a == 0) {
> >       do0();
> >     }
> >   } else {
> >     if (a < 2) {
> >       if (a == 1) {
> >         do1();
> >       }
> >     } else {
> >       if (a == 2) {
> >         do2();
> >       }
> >     }
> >   }
> >
> > This code is inefficient because the check (a == 1) to execute do1() is
> not
> > needed.
> >
> > The reason is that because we already checked that (a >= 1) initially by
> > checking that also  (a < 2) we basically already inferred that (a == 1)
> > without the need of an extra basic block spawned to check if actually (a
> ==
> > 1).
> >
> > The proposed patch addresses this problem by keeping track of already
> > checked bounds in the LowerSwitch algorithm, so that when the time
> arrives
> > to produce a Leaf Block that checks the equality with the case value /
> range
> > the algorithm can decide if that block is really needed depending on the
> > already checked bounds .
> >
> > For example, the above with "a = 1" would work like this:
> >
> > the bounds start as LB: NONE , UB: NONE
> > as (a < 1) is emitted the bounds for the else path become LB: 1 UB: NONE.
> > This happens because by failing the test (a < 1) we know that the value
> "a"
> > cannot be smaller than 1 if we enter the else branch.
> > After the emitting the check (a < 2) the bounds in the if branch become
> LB:
> > 1 UB: 1. This is because by checking that "a" is smaller than 2 then the
> > upper bound becomes 2 - 1 = 1.
> >
> > When is time to emit the leaf block for "case 1:" we notice that 1 is
> > squeezed exactly in between the LB and UB, which means that if we
> arrived to
> > that block there is no need to emit a block that checks if (a == 1).
> >
> >
> > Please let me know if I did something wrong here and if the reasoning is
> > correct :)
> >
> > The proposed patch also sets as XFAIL the test:
> > "test/Transforms/LowerSwitch/feature.ll".
> > This test checks that the emitted lowered CFG is exactly the one
> expected by
> > the previous algorithm without this optimization. Of course this test
> starts
> > failing when this optimization is added. I added another test that is the
> > same, but is updated with the new expected CFG.
> >
> > Don't know what is the best thing to do with the original "feature.ll"
> test
> > in the case this patch gets accepted. Should we remove it or keep it as a
> > reference?
> >
> > Cheers,
> > Marcello
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140610/6b2b084b/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: optimized_lower_switch_v3.patch
Type: application/octet-stream
Size: 13785 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140610/6b2b084b/attachment.obj>