[Mesa-dev] [PATCH] mesa/st: glsl_to_tgsi: Dissolve arrays who's elements are only accessed directly

[Timothy Arceri]

On 28/11/17 01:01, Gert Wollny wrote:
> Am Montag, den 27.11.2017, 12:14 +0100 schrieb Gert Wollny:
>> Am Sonntag, den 26.11.2017, 18:01 -0800 schrieb Eric Anholt:
>>>
>>>
>>> This looks like something that should be done with (at most) a
>>> small change to opt_array_splitting.cpp, rather than reimplementing
>>> it here.
>>
>> It seems that the reason the arrays are not split there is that in
>> the case of the given piglits, the arrays are accessed in loops, and
>> only because the loops are completely unrolled (on r600), the array
>> elements are then no longer addressed indirectly and can be split.
> 
> 
>>
>> Since I have not yet looked at that part of the code I wonder now
>> whether it one can simply move the "optimize_split_arrays" step past
>> the loop unrolling, or whether one should duplicate that step.
> 
> Okay, now I see that the optimization is run various times, so it is
> not clear to me why the array is not split in
> glsl-1.50/execution/geometry/max-input-components
> 
> There the array that doesn't get split is declared like
> 
> in blk {
>    float f[min(gl_MaxGeometryInputComponents,
>                gl_MaxVertexOutputComponents)];
> } ins[];
> 
> The typical IR for the element access then looks like this
> 
> (if
>   (expression bool != (array_ref
>                        (record_ref
>                         (array_ref
>                          (var_ref ins)
>                           (constant int (4))
>                          )
>                        f)
>                       (constant int (81)) )
>                         (constant float (4082.000000)) ) (
>          (assign  (x) (var_ref ok)  (constant bool (0)) )
>        )
>        ())
> 
> in TGSI this is.
> 
>    FSNE TEMP[851].x := ARRAY(1)[81].xxxx, IMM[83].xxxx
>   
> i.e. direct addressing.
> 
> 
> If I understand the comments in opt_array_splitting, arrays of arrays
> are not properly handled there. Curretly, I don't see how I can fix
> this, because I simply didn't look at any code there before.

There is more to it than that. We don't even split varyings at all 
currently.

"This skips uniform/varying arrays, which would need careful handling 
due to their ir->location fields tying them to the GL API and other 
shader stages."

This is enforced in get_variable_entry() via:

    if (var->data.mode != ir_var_auto &&
        var->data.mode != ir_var_temporary)
       return NULL;

As per my previous email I don't think there is much value in putting a 
bunch of work into this pass.

> 
> Best,
> Gert
>