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

[Nicolai Hähnle]

On 03.11.2016 13:02, Marek Olšák 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?
>
> Nicolai, there are a ton of TCS piglit failures if I drop this patch
> and I'm not interested in fixing those now, so I'll keep the patch
> with this comment instead:
>
>       /* Lowering branches with TCS output accesses breaks many piglit tests,
>        * so don't touch them for now.
>        */

I'd still like to understand that eventually, but for now this is fine 
with me.

Nicolai

>
> Marek
>