[Mesa-dev] [PATCH] glsl/dead_builin_varyings: Fix gl_FragData array lowering

[Tapani Pälli]

On 11/30/2015 10:14 AM, Iago Toral wrote:
> On Mon, 2015-11-30 at 09:56 +0200, Tapani Pälli wrote:
>> Hi;
>>
>> On 11/27/2015 02:14 PM, Iago Toral Quiroga wrote:
>>> The current implementation looks for array dereferences on gl_FragData and
>>> immediately then proceeds to lower them, however this is not enough because
>>> we can have array access on vector variables too, like in this code:
>>>
>>> out vec4 color;
>>> void main()
>>> {
>>>      int i;
>>>      for (i = 0; i < 4; i++)
>>>         color[i] = 1.0;
>>> }
>>>
>>> Fix it by making sure that the actual variable being dereferenced is an array.
>>>
>>> Fixes a crash in:
>>> spec/arb_gpu_shader_fp64/execution/built-in-functions/fs-ldexp-dvec4.shader_test
>>
>> Instead of asserting in IR validation now we assert in backend because
>> array 'should have been lowered' .. is there some other patches that
>> should be bundled together with this to make it work?
>
> Yes, notice that this is a fp64 test, which is not implemented in i965
> yet. There is a development branch here if you want to try it:
>
> https://github.com/Igalia/mesa/tree/i965-fp64-v5-testing
>
> and with that branch (which includes this patch) the test does not crash
> any more (it still fails though).

Right, I wanted to make sure we don't assert because of this change but 
something else as this patch disables the pass to happen for such case 
also without fp64 functionality. Now I realize the assert I get is from 
some IR op not lowered which would happen anyway without this change as 
it's not implemented yet. All fine!

Reviewed-by: Tapani Pälli <tapani.palli at intel.com>

> Iago
>
>>> ---
>>>    src/glsl/opt_dead_builtin_varyings.cpp | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/glsl/opt_dead_builtin_varyings.cpp b/src/glsl/opt_dead_builtin_varyings.cpp
>>> index 68b70ee..5387113 100644
>>> --- a/src/glsl/opt_dead_builtin_varyings.cpp
>>> +++ b/src/glsl/opt_dead_builtin_varyings.cpp
>>> @@ -85,7 +85,7 @@ public:
>>>       {
>>>          ir_variable *var = ir->variable_referenced();
>>>
>>> -      if (!var || var->data.mode != this->mode)
>>> +      if (!var || var->data.mode != this->mode || !var->type->is_array())
>>>             return visit_continue;
>>>
>>>          if (this->find_frag_outputs && var->data.location == FRAG_RESULT_DATA0) {
>>>
>>
>
>