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

Christoph Bumiller e0425955 at student.tuwien.ac.at
Thu Nov 8 10:08:50 PST 2012


On 08.11.2012 18:16, Tom Stellard wrote:
> On Thu, Nov 08, 2012 at 05:45:00PM +0100, Christoph Bumiller wrote:
>> 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.
> 
> I don't quite understand this solution, could you show me a simple
> example TGSI program.
> 

DCL TEMP[0..3] = "array" without indirect access (registers)
DCL TEMP[4..12] = indirectly accessed array
DCL TEMP[12..20] = another indirectly accessed array
DCL IMM[0] { -2, 0, 0, 0 }

fill with data:
MOV TEMP[4], IN[1]
MOV TEMP[5], IN[2]
etc.

indirect move with address (IN[0].x - 2) from array TEMP[4..12] to output:
UADD TEMP[0].x, IN[0].x, IMM[0].x <- added here explicitly instead of
using (4 - IMM[0].x == 2) in the MOV below
UARL ADDR[0].x, TEMP[0].x
MOV OUT[1], TEMP[ADDR[0].x + 4] <- must be 4 here to identify the
array/range (iff there is indirect addressing)

Of course we can get rid of the ARL stuff, too, at some point.

> -Tom
> 
>>
>> 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
>>>>
>>



More information about the mesa-dev mailing list