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

Wed Jun 24 23:26:57 PDT 2015

Hi,

The ARB asm programs only allow indirect addressing into the constant buffer.

Only GLSL allows indirect addressing everywhere and GLSL-to-TGSI
already generates proper array declarations, so we should be fine.

I think we can just assume that indirect addressing into TEMP, INPUT,
or OUTPUT with ArrayID==0 is an invalid instruction and should be
disallowed and guarded by assert(). Of course, all drivers must
support PIPE_SHADER_CAP_TGSI_ANY_INOUT_DECL_RANGE first, so that
GLSL-to-TGSI can use the array declarations.

Marek

On Thu, Jun 25, 2015 at 5:17 AM, Rob Clark <robdclark at gmail.com> wrote:
> I'd be a big fan of letting it go..  if needed for old ARB shader
> stuff we can give entire IN/OUT file ArrayID==1.. if someone points me
> in the right direction I don't mind going and poking at it since the
> whole assuming-the-worst thing if ArrayID==0 is not so nice for
> register allocation..
>
> BR,
> -R
>
> On Wed, Jun 24, 2015 at 10:38 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>> I thought this was needed by something in the gl state tracker. relative
>> addressing with ARB_vp maybe, can't remember...
>> I dunno if all places where this is used are fixed up it can go.
>>
>> Roland
>>
>>
>> Am 25.06.2015 um 01:31 schrieb Rob Clark:
>>> tgsi.rst currently says:
>>>
>>> ----
>>> If no ArrayID is specified with an indirect addressing operand the whole
>>> register file might be accessed by this operand. This is strongly discouraged
>>> and will prevent packing of scalar/vec2 arrays and effective alias analysis.
>>> ----
>>>
>>> I'd be pretty happy to remove it and replace it w/ something to the
>>> effect of indirect addressing where no ArrayID is specified is
>>> undefined :-)
>>>
>>> BR,
>>> -R
>>>
>>> On Wed, Jun 24, 2015 at 6:17 PM, Marek Olšák <maraeo at gmail.com> wrote:
>>>> Indirect addressing without the ArrayID is undefined in the general
>>>> case. You need the ArrayID to be able to tell where the array
>>>> declaration starts and what its semantic name is.
>>>>
>>>> Marek
>>>>
>>>> On Wed, Jun 24, 2015 at 8:35 PM, Rob Clark <robdclark at gmail.com> wrote:
>>>>> 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.
>>>>>
>>>>> ok.. hmm, I guess tgsi.rst needs an update then..
>>>>>
>>>>>> 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
>>>>>
>>>>> ok, in principle (after fixing ttn based on the assumption that we
>>>>> won't get indirect access if ArrayID==0, and now that tgsi f/e is
>>>>> dropped) freedreno should be ok for array in/out's..  I'll enable the
>>>>> cap and see what happens after fixing ttn and debugging some indirect
>>>>> issues (seems either there is something I don't understand about the
>>>>> hw yet, or maybe an a4xx bug.. right now shaders that I think should
>>>>> work aren't and I'm in android hell trying to get some more traces
>>>>> from blob :-/)
>>>>>
>>>>> BR,
>>>>> -R
>>>>>
>>>>>> 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
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>