[Mesa-dev] [PATCH 2/4] gallium: Add PIPE_SHADER_CAP_INDIRECT_TEMP_ARRAY_ADDR

Thu Nov 8 08:45:00 PST 2012

On 08.11.2012 16:03, Tom Stellard wrote:
> On Thu, Nov 08, 2012 at 12:59:06PM +0100, Christoph Bumiller wrote:
>> On 08.11.2012 01:48, Brian Paul wrote:
>>> On 11/05/2012 01:14 PM, Tom Stellard wrote:
>>>> From: Tom Stellard<thomas.stellard at amd.com>
>>>>
>>>> ---
>>>>   src/gallium/docs/source/screen.rst           | 2 ++
>>>>   src/gallium/drivers/r600/r600_pipe.c         | 2 ++
>>>>   src/gallium/drivers/radeonsi/radeonsi_pipe.c | 1 +
>>>>   src/gallium/include/pipe/p_defines.h         | 3 ++-
>>>>   4 files changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/gallium/docs/source/screen.rst
>>>> b/src/gallium/docs/source/screen.rst
>>>> index 6c89171..60fdc3a 100644
>>>> --- a/src/gallium/docs/source/screen.rst
>>>> +++ b/src/gallium/docs/source/screen.rst
>>>> @@ -211,6 +211,8 @@ to be 0.
>>>>     samplers.
>>>>   * ``PIPE_SHADER_CAP_PREFERRED_IR``: Preferred representation of the
>>>>     program.  It should be one of the ``pipe_shader_ir`` enum values.
>>>> +* ``PIPE_SHADER_CAP_INDIRECT_TEMP_ARRAY_ADDR``: Whether indirect
>>>> addressing
>>>> +  of the temporary array file is supported.
>>>
>>> Is it correct to say that TGSI_FILE_TEMPORARY_ARRAY is only used when
>>> this cap is true?
>>>
>> It's really not used or supported at all.
>>
>>> I think the docs for TGSI_FILE_TEMPORARY_ARRAY leave something to be
>>> desired.  Maybe you or someone else could try to add more documentation
>>> for that.
>>>
>>
>> In my opinion this file should be *removed*, along with IMMEDIATE_ARRAY.
>>
>> It's pointless: If you have multiple (indirectly accessed) arrays, you
>> need a way to distinguish them anyway, and if you have that, you can
>> just as well apply it to the normal TEMPORARY file.
>>
> 
> Do you have any ideas on how to use distinguish between arrays while
> still using the temporary file?
> 
> What I'm looking for is a way to distinguish between registers that can be
> indirectly accessed versus those that can't.  Currently, we cannot perform

Ideally you'd want to distinguish individual arrays, too, to open more
possibilities for optimization.

What we envisioned was simply using multiple declarations for any file
that has arrays, one declaration per array, and identify the array that
is being accessed indirectly by the immediate offset of the source
register. The remaining part of the immediate offset can be added to the
address register by a simple ADD immediate, which is easly optimized
away by the backend.

With this, drivers don't need to be changed at all, and you get proper
identification of arrays for all files, that is also INPUT and OUTPUT.

> proper register allocation on any shader that uses indirect addressing,
> since it is impossible to predict which register an indirect read/write
> may access.  This is especially problematic in the case of a large shader
> that declares an array with a very small numbers of items.
> 
> Maybe TGSI_FILE_TEMPORARY_ARRAY is not the best register file to use
> in this case, but if not I think we should come up with a new one.
> 
> -Tom
> 
>>> In the other patch, when you query
>>> PIPE_SHADER_CAP_INDIRECT_TEMP_ARRAY_ADDR it seems you're really just
>>> querying whether the TGSI_FILE_TEMPORARY_ARRAY file is supported,
>>> regardless of whether or not it's dynamically indexed, right?
>>>
>>> Maybe a better name for the cap would be PIPE_SHADER_CAP_TEMP_ARRAY to
>>> indicate whether TGSI_FILE_TEMPORARY_ARRAY is accepted by the driver.
>>>  The expectation is that it would typically be indexed with an address
>>> register.  I assume "INDIRECT" means dynamic indexing.
>>>
>>>
>>>>
>>>>
>>>>   .. _pipe_compute_cap:
>>>> diff --git a/src/gallium/drivers/r600/r600_pipe.c
>>>> b/src/gallium/drivers/r600/r600_pipe.c
>>>> index b5280e3..abe7ad7 100644
>>>> --- a/src/gallium/drivers/r600/r600_pipe.c
>>>> +++ b/src/gallium/drivers/r600/r600_pipe.c
>>>> @@ -563,6 +563,8 @@ static int r600_get_shader_param(struct
>>>> pipe_screen* pscreen, unsigned shader, e
>>>>               return PIPE_SHADER_IR_TGSI;
>>>>           }
>>>>       }
>>>> +    case PIPE_SHADER_CAP_INDIRECT_TEMP_ARRAY_ADDR:
>>>> +        return 0;
>>>>       return 0;
>>>>   }
>>>>
>>>> diff --git a/src/gallium/drivers/radeonsi/radeonsi_pipe.c
>>>> b/src/gallium/drivers/radeonsi/radeonsi_pipe.c
>>>> index fa16f4c..1a1235f 100644
>>>> --- a/src/gallium/drivers/radeonsi/radeonsi_pipe.c
>>>> +++ b/src/gallium/drivers/radeonsi/radeonsi_pipe.c
>>>> @@ -463,6 +463,7 @@ static int r600_get_shader_param(struct
>>>> pipe_screen* pscreen, unsigned shader, e
>>>>       case PIPE_SHADER_CAP_INDIRECT_OUTPUT_ADDR:
>>>>       case PIPE_SHADER_CAP_INDIRECT_TEMP_ADDR:
>>>>       case PIPE_SHADER_CAP_INDIRECT_CONST_ADDR:
>>>> +    case PIPE_SHADER_CAP_INDIRECT_TEMP_ARRAY_ADDR:
>>>>           return 0;
>>>>       case PIPE_SHADER_CAP_INTEGERS:
>>>>           return 1;
>>>> diff --git a/src/gallium/include/pipe/p_defines.h
>>>> b/src/gallium/include/pipe/p_defines.h
>>>> index 184136e..0841528 100644
>>>> --- a/src/gallium/include/pipe/p_defines.h
>>>> +++ b/src/gallium/include/pipe/p_defines.h
>>>> @@ -533,7 +533,8 @@ enum pipe_shader_cap
>>>>      PIPE_SHADER_CAP_SUBROUTINES = 16, /* BGNSUB, ENDSUB, CAL, RET */
>>>>      PIPE_SHADER_CAP_INTEGERS = 17,
>>>>      PIPE_SHADER_CAP_MAX_TEXTURE_SAMPLERS = 18,
>>>> -   PIPE_SHADER_CAP_PREFERRED_IR = 19
>>>> +   PIPE_SHADER_CAP_PREFERRED_IR = 19,
>>>> +   PIPE_SHADER_CAP_INDIRECT_TEMP_ARRAY_ADDR = 20
>>>>   };
>>>>
>>>>   /**
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>