[Mesa-dev] [RFC] gallium/hud: fix issue w/ tgsi_to_nir

[Connor Abbott]

On Sat, Jun 27, 2015 at 6:48 AM, Rob Clark <robdclark at gmail.com> wrote:
> I had a quick look at enabling this for freedreno.. although looks
> like it is going to take some work in tgsi_to_nir and/or nir.. the
> problem is we end up w/ a register array (rather than variable array
> like we do for TEMP arrays).. ie. for something copying from a TEMP
> array to an OUT array, we end up like:
>
>     vec4 ssa_35 = intrinsic load_var () (arr_2[0]) ()
>     vec4 ssa_36 = fmov ssa_35
>     r1[0] = fmov ssa_36
>     vec4 ssa_37 = intrinsic load_var () (arr_2[1]) ()
>     vec4 ssa_38 = fmov ssa_37
>     r1[1] = fmov ssa_38
>     vec4 ssa_39 = intrinsic load_var () (arr_2[2]) ()
>     vec4 ssa_40 = fmov ssa_39
>     r1[2] = fmov ssa_40
>     vec4 ssa_41 = intrinsic load_var () (arr_2[3]) ()
>     vec4 ssa_42 = fmov ssa_41
>     r1[3] = fmov ssa_42
>     vec4 ssa_43 = fmov r2
>     r0 = fmov ssa_43
>     intrinsic store_output (r0) () (0)
>     intrinsic store_output (r1[0]) () (1)
>     intrinsic store_output (r1[1]) () (2)
>     intrinsic store_output (r1[2]) () (3)
>     intrinsic store_output (r1[3]) () (4)
>
> the r1[] array ends up not getting converted into SSA and hitting an
> assert when we try to lower to scalar.. I'm not really sure the best
> way to handle this

Well, the frontend shouldn't really be generating register arrays...
they're for backends, not frontends, and it's curently impossible to
eliminate them without extra information about the stride that I've
been hesitant to add, and even then they're more of a pain for
optimization passes. I'm not sure the exact place where ttn is
emitting a register array, but if you fix it to use a variable
instead, vars_to_ssa should clean it up.

>
> BR,
> -R
>
> On Wed, Jun 24, 2015 at 12:31 PM, Marek Olšák <maraeo at gmail.com> wrote:
>> Yes, ArrayID != 0 means it's an array, otherwise it's just a compact
>> way to add declarations.
>>
>> I recently added the array support for inputs and outputs, we just
>> need to enable it on non-radeon non-swrast drivers:
>> http://cgit.freedesktop.org/mesa/mesa/commit/?id=b6ebe7eabf54936a02acc0968e718e0c264a73f5
>>
>> It would be nice to enable the arrays on all drivers instead of
>> working around it indefinitely.
>>
>> Marek
>>
>> On Wed, Jun 24, 2015 at 1:31 PM, Rob Clark <robdclark at gmail.com> wrote:
>>> Ok, I *thought* we didn't get ArrayID on IN/OUT, but only TEMP?  If it
>>> is safe to assume that we always get ArrayID that makes it much
>>> easier.
>>>
>>> BR,
>>> -R
>>>
>>> On Wed, Jun 24, 2015 at 5:39 AM, Marek Olšák <maraeo at gmail.com> wrote:
>>>> It's not an array, because the ArrayID is 0. It's a valid non-array
>>>> declaration. If any TGSI user doesn't understand it, that user should
>>>> be fixed.
>>>>
>>>> Marek
>>>>
>>>> On Tue, Jun 23, 2015 at 3:20 PM, Rob Clark <robdclark at gmail.com> wrote:
>>>>> From: Rob Clark <robclark at freedesktop.org>
>>>>>
>>>>> Ok, so actually there is a ttn issue here to fix as well.. but it
>>>>> brought up a question in my mind.  When ttn sees something like
>>>>>
>>>>>   DCL IN[0..1]
>>>>>
>>>>> it will treat that as an array (which in the end will result in
>>>>> constraints about where the registers get allocated.  Which is not
>>>>> really ideal.
>>>>>
>>>>> With glsl we don't actually get input arrays (but instead a bunch
>>>>> of MOV's to a TEMP array) currently.  So I'm not quite sure how
>>>>> an actual input array should look.  (But my preference would be
>>>>> IN[a..b] for arrays and IN[c] otherwise)
>>>>> ---
>>>>>  src/gallium/auxiliary/hud/hud_context.c | 3 ++-
>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/src/gallium/auxiliary/hud/hud_context.c b/src/gallium/auxiliary/hud/hud_context.c
>>>>> index 6a124f7..2b6d3a7 100644
>>>>> --- a/src/gallium/auxiliary/hud/hud_context.c
>>>>> +++ b/src/gallium/auxiliary/hud/hud_context.c
>>>>> @@ -1163,7 +1163,8 @@ hud_create(struct pipe_context *pipe, struct cso_context *cso)
>>>>>     {
>>>>>        static const char *vertex_shader_text = {
>>>>>           "VERT\n"
>>>>> -         "DCL IN[0..1]\n"
>>>>> +         "DCL IN[0]\n"
>>>>> +         "DCL IN[1]\n"
>>>>>           "DCL OUT[0], POSITION\n"
>>>>>           "DCL OUT[1], COLOR[0]\n" /* color */
>>>>>           "DCL OUT[2], GENERIC[0]\n" /* texcoord */
>>>>> --
>>>>> 2.4.3
>>>>>
>>>>> _______________________________________________
>>>>> mesa-dev mailing list
>>>>> mesa-dev at lists.freedesktop.org
>>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev