[PATCH] Improving LowerSwitch behavior for contiguous ranges

[Marcello Maggioni]

Here is a patch with the original test 'feature.ll' modified to pass on the
new implementation

Marcello


2014-06-10 17:06 GMT+01:00 Marcello Maggioni <hayarms at gmail.com>:

> The new test is basically the old test updated to pass on the optimized
> algorithm.
>
> I'll produce a patch with the new test renamed as the old test.
>
> Marcello
>
>
> 2014-06-10 16:19 GMT+01:00 David Blaikie <dblaikie at gmail.com>:
>
> 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?
>>
>> Most likely we should just change the original to test the new
>> behavior, and not introduce a new test. If the original test doesn't
>> test all cases of the new behavior and can't be reasonably made to do
>> so, we could introduce a new test for those new cases (while keeping
>> the old one covering the existing cases, but updated to verify the new
>> optimization you've implemented is functioning)
>>
>> - David
>>
>> >
>> > 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/00320493/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: optimized_lower_switch_v2.patch
Type: application/octet-stream
Size: 13604 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140610/00320493/attachment.obj>