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.stell...@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. -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@lists.freedesktop.org > >>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev > >> > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
