[Mesa-dev] [PATCH 6/6] glsl: Generate IR for switch statements

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/24/2011 05:11 PM, Dan McCabe wrote:
> On 06/24/2011 01:17 PM, Dan McCabe wrote:
>> On 06/20/2011 03:50 PM, Ian Romanick wrote:
>>> -----BEGIN PGP SIGNED MESSAGE-----
>>> Hash: SHA1
>>>
>>> On 06/17/2011 05:43 PM, Dan McCabe wrote:
>>>> Beware! Here be dragons!
>>>>
>>>>
>>> I think this will generate the wrong code for:
>>>
>>>     for (i = 0; i<  10; i++) {
>>>         switch (i) {
>>>             default: continue;
>>>         }
>>>     }
>>>
>>> It seems like that will generate some approximation of:
>>>
>>>     for (i = 0; i<  10; i++) {
>>>         do {
>>>             continue;
>>>             break;
>>>         } while (true);
>>>     }
>>>
>>> Right?  This is why I had originally tracked loops and switch-statements
>>> in a single stack.  In this situation, you'd want to set a "do a
>>> continue" flag in the switch-statement and emit a break (from the
>>> switch).  Something like:
>>>
>>>
>>>     for (i = 0; i<  10; i++) {
>>>         bool do_cont = false;
>>>
>>>         do {
>>>             do_cont = true;
>>>             break;
>>>             break;
>>>         } while (true);
>>>
>>>         if (do_cont)
>>>             continue;
>>>     }
>>>
>>>
>> Yikes! Looks like you tripped over one of those dragons :(.
>>
>> Using a do_cont variable (or similar device) feels kludgey to me. I've
>> been mulling this over for the last couple of days while I did other
>> things and I think the right way to do this might be to get rid of the
>> use of "loop" altogether.
>>
>> But the devil is in the details, which I haven't worked out yet. Going
>> back to the unified loop/switch stack might be needed, though.
>>
>> cheers, danm
>>
> 
> All is not lost! (But you already knew that :)
> 
> One of the motivations of using the loop device is that it enabled the
> relatively unmodified infrastructure for break statements. But let's get
> rid of the break device and see where we go.
> 
> Without xlating switch stmts into loops, we can still retain the
> test_val_tmp and is_fallthru_tmp temporaries (in fact, we need to;
> nothing changes there since that is how case labels are managed).
> 
> However, the standard break processing no longer applies, since we are
> not in a loop (for the switch stmt in any event). If we introduce a bool
> temporary called is_break_tmp and initialize it to false, then when we
> encounter a break stmt, we set is_break_tmp to true. Now we must guard
> the case statement with both fallthru_var and is_break_var. If
> is_break_tmp was set, we simply clear is_fallthru_tmp right after all
> case tests to false.
> 
> Note that this approach does require maintaining a loop/switch stack as
> the code originally did so that we can tell if a break statement is
> associate with a loop or with a switch stmt. Only if the top of the
> stack indicates that switch is being processed will the above break
> processing be invoked.
> 
> Looking at a translation of my canonical example:
> 
>     switch (expr) {
>     case c0:
>     case c1:
>          stmt0;
>     case c2:
>     case c3:
>          stmt1;
>          break;
>     case c4:
>     default:
>          stmt2;
>     }
> 
> We can translated this into:
> 
>     int test_val_tmp = expr;
>     bool is_fallthru_tmp = false;
>     bool is_break_tmp = false;
> 
>     if (test_val_tmp == c0) {
>        is_fallthru_tmp = true;
>     }
>     if (test_val_tmp == c1) {
>        is_fallthru_tmp = true;
>     }
>     if (is_break_tmp) {
>        is_fallthru_tmp = false;
>     }
>     if (is_fallthru_tmp) {
>        stmt0;
>     }
> 
>     if (test_val_tmp == c2) {
>        is_fallthru_tmp = true;
>     }
>     if (test_val_tmp == c3) {
>        is_fallthru_tmp = true;
>     }
>     if (is_break_tmp) {
>        is_fallthru_tmp = false;
>     }
>     if (is_fallthru_tmp) {
>        stmt1;
>        is_break_tmp = true; // break stmt
>     }
> 
>     if (test_val_tmp == c4) {
>        is_fallthru_tmp = true;
>     }
>     is_fallthru_tmp = true; // default
>     if (is_break_tmp) {
>        is_fallthru_tmp = false;
>     }
>     if (is_fallthru_tmp) {
>        stmt2;
>     }
> 
> Comparing this to what we did previously, the things that change in this
> code are:
> 1) no enclosing loop
> 2) create a bool is_break_tmp initialized to false
> 3) after all case labels and before each case statement list, clear
> is_fallthru_tmp when is_break_tmp
> 4) set is_break_tmp for each break statement

That sounds very reasonable.

> And maintain the loop/switch stack again as in the original code.
> 
> I'll get this cranked out on Monday and resubmit for review. If anyone
> has concerns or questions about what I'm doing here, please let me know.
> 
> cheers, danm

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk4I+DEACgkQX1gOwKyEAw9bugCgiAH72AG2WnDQ0pvQAxMHTShd
NSgAn13bahn00ZW5vTxmnic2tWWG2+DS
=93jZ
-----END PGP SIGNATURE-----