[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