[Mesa-dev] [PATCH 3/7] glsl/lower_if: don't lower branches touching tess control outputs

[Marek Olšák]

On Thu, Nov 3, 2016 at 12:01 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Wed, Nov 2, 2016 at 6:55 PM, Marek Olšák <maraeo at gmail.com> wrote:
>> On Wed, Nov 2, 2016 at 2:55 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>>> On 29.10.2016 01:13, Marek Olšák wrote:
>>>>
>>>> From: Marek Olšák <marek.olsak at amd.com>
>>>>
>>>> ---
>>>>  src/compiler/glsl/ir_optimization.h           |  3 ++-
>>>>  src/compiler/glsl/lower_if_to_cond_assign.cpp | 23
>>>> ++++++++++++++++++++---
>>>>  src/compiler/glsl/test_optpass.cpp            |  2 +-
>>>>  src/mesa/drivers/dri/i965/brw_link.cpp        |  2 +-
>>>>  src/mesa/program/ir_to_mesa.cpp               |  3 ++-
>>>>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp    |  3 ++-
>>>>  6 files changed, 28 insertions(+), 8 deletions(-)
>>>>
>>> [snip]
>>>>
>>>> @@ -105,20 +109,33 @@ check_ir_node(ir_instruction *ir, void *data)
>>>>     case ir_type_call:
>>>>     case ir_type_discard:
>>>>     case ir_type_loop:
>>>>     case ir_type_loop_jump:
>>>>     case ir_type_return:
>>>>     case ir_type_emit_vertex:
>>>>     case ir_type_end_primitive:
>>>>     case ir_type_barrier:
>>>>        v->found_unsupported_op = true;
>>>>        break;
>>>> +
>>>> +   case ir_type_dereference_variable: {
>>>> +      ir_variable *var =
>>>> ir->as_dereference_variable()->variable_referenced();
>>>> +
>>>> +      /* Tess control shader outputs are like shared memory with complex
>>>> +       * side effects, so treat it that way.
>>>> +       */
>>>> +      if (v->stage == MESA_SHADER_TESS_CTRL &&
>>>> +          var->data.mode == ir_var_shader_out)
>>>> +         v->found_unsupported_op = true;
>>>> +      break;
>>>> +   }
>>>> +
>>>
>>>
>>> I don't understand this rationale. What side effects are there, and can you
>>> give an example where the transformation would be wrong?
>>
>> Well I can't think of a case now where it would break something.
>
> Not sure how dereference_variable is used... what happens if you do
>
> out[gl_InvocationId] = asdf
>
> does that use a dereference_variable for the out? If so, this could be
> problematic, e.g.
>
> if (foo)
>   out[gl_InvocationId] = asdf
>
> tmp = out[0];
>
> You can't really "fix" that in any reasonable way... Or perhaps I'm
> confused about how this pass works.

It works like this:

if (c)
   x = a;
else
   x = b;

is lowered to:

x = c ? a : x;
x = !c ? b : x;

Marek