[llvm-commits] [LLVM, loop-unswitch, bugfix for #11429] Wrong behaviour for switches.

[Dan Gohman]

Hello,

Non-trivial loop unswitching is often not worthwhile because of the
code size increases (especially since LLVM doesn't have other
advanced loop optimizations). It's even possible that this bug is
accidentally helping the code by preventing it from unswitching too
much in the presence of switches. Do you have an idea on what impact
this patch has on code size, and performance, in general?

It's non-obvious why releaseMemory() isn't releasing all of its
state. I guess it's remembering things about the newly created loop?
Can you add some comments about this?

CloneUnswitchedVals doesn't actually need to iterate over the
instructions in the block to find the SwitchInst. If there's a
SwitchInst present, it'll be the Terminator instruction.

Thanks,

Dan

On Dec 2, 2011, at 12:31 AM, Stepan Dyatkovskiy wrote:

> 
> ping.
> 
> -Stepan.
> Stepan Dyatkovskiy wrote:
>> ping.
>> 
>> -Stepan.
>> 
>> Stepan Dyatkovskiy wrote:
>>> ping.
>>> 
>>> -Stepan.
>>> 
>>> Stepan Dyatkovskiy wrote:
>>>> ping.
>>>> 
>>>> -Stepan
>>>> 
>>>> Stepan Dyatkovskiy wrote:
>>>>> Hi all. Please find the patch in attachment for review. Regression tests
>>>>> are attached in separated patch.
>>>>> 
>>>>> Short tests description:
>>>>> 
>>>>> 2011-11-18-SimpleSwitch.ll
>>>>> Check simple that simple switch will totally unswitched:
>>>>> 
>>>>> for (...)
>>>>> switch(c) {
>>>>> case 0: inc(); break;
>>>>> case 1: dec(); break;
>>>>> default: goto loop_exit;
>>>>> }
>>>>> 
>>>>> loop_exit:
>>>>> ...
>>>>> 
>>>>> Result of processing should be 2 additional loops for c == 0 and for c
>>>>> == 1.
>>>>> 
>>>>> 
>>>>> 2011-11-18-TwoSwitches.ll
>>>>> Check that second switch will unswitched too. Check that switches will
>>>>> not unswitched again in new loop:
>>>>> 
>>>>> Initially we have the next:
>>>>> 
>>>>> for (...) {
>>>>> switch(c) {
>>>>> case 0: inc(); break;
>>>>> }
>>>>> switch(d) {
>>>>> case 0: inc(); break;
>>>>> }
>>>>> }
>>>>> 
>>>>> After optimization we should got 3 additional loops: when (c == 0&&    d
>>>>> == 0), when (c == 0&&    d != 0) and when (c != 0&&    d == 0). Original
>>>>> loop will activated for (c != 0&&    d != 0):
>>>>> 
>>>>> if (c == 0&&    d == 0) {
>>>>> for (...) ... // All is clear here. Two "switch(0)" instructions.
>>>>> } else if (c == 0&&    d != 0) {
>>>>> for (...) {
>>>>> switch (0) { // c == 0
>>>>> case 0: inc(); break;
>>>>> }
>>>>> switch (d) {
>>>>> case 0: goto unreachable; // CHECK: That it will not unswitched
>>>>> // again, since it looks "trivial",
>>>>> // see LoopUnswitch::IsTrivialUnswitchCondition
>>>>> }
>>>>> }
>>>>> } else if (c != 0&&    d == 0) {
>>>>> // the same...
>>>>> ...
>>>>> } else { // if (c != 0&&    d != 0)
>>>>> // Original totally unswitched loop:
>>>>> switch (c) {
>>>>> case 0: goto unreachable;
>>>>> }
>>>>> switch (d) {
>>>>> case 0: goto unreachable;
>>>>> }
>>>>> }
>>>>> 
>>>>> Stepan.
>>>>> 
>>>>> 
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>> 
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits