[PATCH] peephole optimization in switch table lookup: reuse the guarding table comparison if possible

[Erik Eckstein]

I did some investigation and found that we already have such an optimization.
It is InstCombiner::FoldOpIntoPhi(Instruction &I) where I is the "r == 0" cmp instruction.

There are two reasons why it is not done for switch tables:
1) phase ordering: FoldOpIntoPhi must be done before switch table generation
2) FoldOpIntoPhi triggers only if the phi has only one use (seems to be a simple kind of "cost model")

I think that both issues can be solved somehow:
1) Maybe just call FoldOpIntoPhi for phi users (which are cmp) before doing the switch table generation (for this specific phi).
2) Need to investigate if there are any negative effects if there are other phi-users beside a cmp. I have checked the history and there was some code to do it more aggressively, but I don't know the details.

Erik


> On 18 Nov 2014, at 20:07, Hans Wennborg <hans at chromium.org> wrote:
> 
> On Tue, Nov 18, 2014 at 10:08 AM, Hans Wennborg <hans at chromium.org <mailto:hans at chromium.org>> wrote:
>> Hi Erik,
>> 
>> (Haven't looked much at the patch yet, just thinking about the idea
>> itself right now.)
>> 
>> On Tue, Nov 18, 2014 at 7:53 AM, Erik Eckstein <eeckstein at apple.com> wrote:
>>> Hi Hans,
>>> 
>>> I have another patch for the switch table lookup generation. It tries to reuse the generated compare instruction, if there is a comparison agains the default value after the switch.
>>> 
>>> Example:
>>> 
>>> switch (x) {
>>>  case 0: r = 10; break;
>>>  case 1: r = 11; break;
>>>  ...
>>>  default: r = 0; break; // 0 does not appear in any case value.
>>> }
>>> if (r == 0) {
>>>  do_something;
>>> }
>>> 
>>> transforms to:
>>> 
>>> if (x < table_size) {
>>>  r = table[x];
>>> } else {
>>>  r = 0;
>>>  do_something;
>>> }
>> 
>> Interesting. We're basically exploiting that we have extra knowledge
>> about the relationship between x and r. It would be cool if it was a
>> bit broader though, e.g. what if it was "if (r < 10)", and that was
>> only true if we hit the default case in the switch.
>> 
>> My main concern is whether the benefit is worth the complexity this
>> adds to SwitchToLookupTable (or if that's the right place for it).
>> 
>> I also wonder if we could make it broader, based on control-flow and
>> value tracking. For example, if we have:
>> 
>>  vars = phi
>>  if (expr(vars)) {
>>    do stuff
>>  }
>> 
>> If we know that expr(vars) is only true for one of the incoming edges
>> to the phi, could we try to hoist the if-body into that incoming edge?
>> It seems this would cover your example, and could maybe hit more
>> cases.
> 
> I guess I'm thinking that your example isn't really specific to the
> switch transformation. For example, even if we're not building a
> lookup table, your code could still save the conditional branch by
> transforming to:
> 
> switch (x) {
>   case 0: r = 10; break;
>   case 1: r = 11; break;
>   ...
>   default:
>     r = 0;
>     do_something;
>     break; // 0 does not appear in any case value.
> }

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141119/91d75ff3/attachment.html>