[Mesa-dev] [RFC] gallium/hud: fix issue w/ tgsi_to_nir
Connor Abbott
cwabbott0 at gmail.com
Sat Jun 27 12:54:04 PDT 2015
On Sat, Jun 27, 2015 at 11:39 AM, Rob Clark <robdclark at gmail.com> wrote:
> On Sat, Jun 27, 2015 at 12:57 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
>> 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.
>
> I was thinking about this, since I'd already done the same for TEMP
> file arrays..
>
> How should that end up looking, though, from the perspective of
> store_output? Can store_output reference a variable, or do I have to
> do a move into ssa and then pass the ssa src to store_output
> (effectively duplicating the lowering IN/OUT arrays that tgsi
> currently does in ttn)
You can create an output variable (i.e. one with its data.mode set to
nir_var_shader_out and stored in the outputs array), and
nir_lower_io() will then take care of lowering stores to that variable
(i.e. store_var intrinsics with the variable set to it) to
store_output intrinsics. Note that if you have an indirect output
write (i.e. a store_var intrinsic with an indirect array deref),
you'll start getting store_output_indirect intrinsics, which you can't
get from GLSL since the varying packer lowers away the only possible
way a shader can write to an output indirectly. I'm not sure if you
care about that, though, since this sounds like it's only for some
internal TGSI shaders that Gallium hands you that don't have indirect
accesses.
In fact, this is the way both inputs and outputs work in glsl -> nir
(i.e. first create a variable and then lower it)... it's just that
when Eric wrote ttn, he sidestepped that part. It sounds like in this
situation, though, it's easier to create a variable for each output
array.
>
> BR,
> -R
>
>
>>>
>>> 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
More information about the mesa-dev
mailing list