[PATCH] Improving LowerSwitch behavior for contiguous ranges

[Marcello Maggioni]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140610/c1b2d71e/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: optimized_lower_switch.patch
Type: application/octet-stream
Size: 10618 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140610/c1b2d71e/attachment.obj>