[Mesa-dev] [PATCH 08/10] glsl: fix new gcc6 warnings

[Rob Clark]

On Tue, Feb 16, 2016 at 2:32 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 02/16/2016 10:58 AM, Rob Clark wrote:
>> src/compiler/glsl/lower_discard_flow.cpp:79:1: warning: ‘ir_visitor_status {anonymous}::lower_discard_flow_visitor::visit_enter(ir_loop_jump*)’ defined but not used [-Wunused-function]
>>  lower_discard_flow_visitor::visit_enter(ir_loop_jump *ir)
>>  ^~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Note, not sure if this was a latent bug?  Could be that was intended to
>> override visit(ir_loop_jump *)?
>
> I'll wager that is correct, and the bug has existed since day 1. :(
>
> To hit the bug, you'd need a loop with both a discard and a continue.  I
> suspect that is a rare combination.  To observe that the bug had been
> hit, you'd have to use a derivative (either via dFdx() and friends or
> texture()) in a particular way.  I'll have to try to think of a test case.

so, I ended up just hacking in a _mesa_print_ir() before/after
lower_discard_flow()..  not really a re-usable test, but enough for me
to verify what was going on.

> It might be easier to just add a unit test.  We know that
>
>    for (int i = 0; i < x; i++) {
>       if (z)
>          continue;
>
>       if (y)
>          discard;
>    }
>
> should get transformed to
>
>    for (int i = 0; i < x; i++) {
>       if (z) {
>          if (discarded)
>             break;

current state is that we miss this 'if (discarded) break;'  Renaming
that fxn to 'visit' instead of 'visit_enter' so it actually overrides
something fixes that.

BR,
-R


>          continue;
>       }
>
>       if (y) {
>          discarded = true;
>          discard;
>       }
>
>       if (discarded)
>          break;
>    }
>
>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>> ---
>>  src/compiler/glsl/lower_discard_flow.cpp | 12 ------------
>>  1 file changed, 12 deletions(-)
>>
>> diff --git a/src/compiler/glsl/lower_discard_flow.cpp b/src/compiler/glsl/lower_discard_flow.cpp
>> index 9d0a56b..bdb96b4 100644
>> --- a/src/compiler/glsl/lower_discard_flow.cpp
>> +++ b/src/compiler/glsl/lower_discard_flow.cpp
>> @@ -63,7 +63,6 @@ public:
>>     }
>>
>>     ir_visitor_status visit_enter(ir_discard *ir);
>> -   ir_visitor_status visit_enter(ir_loop_jump *ir);
>>     ir_visitor_status visit_enter(ir_loop *ir);
>>     ir_visitor_status visit_enter(ir_function_signature *ir);
>>
>> @@ -76,17 +75,6 @@ public:
>>  } /* anonymous namespace */
>>
>>  ir_visitor_status
>> -lower_discard_flow_visitor::visit_enter(ir_loop_jump *ir)
>> -{
>> -   if (ir->mode != ir_loop_jump::jump_continue)
>> -      return visit_continue;
>> -
>> -   ir->insert_before(generate_discard_break());
>> -
>> -   return visit_continue;
>> -}
>> -
>> -ir_visitor_status
>>  lower_discard_flow_visitor::visit_enter(ir_discard *ir)
>>  {
>>     ir_dereference *lhs = new(mem_ctx) ir_dereference_variable(discarded);
>>
>