[Mesa-dev] [PATCH 2/4] gallium: Add PIPE_SHADER_CAP_INDIRECT_TEMP_ARRAY_ADDR
Tom Stellard
tom at stellard.net
Thu Nov 8 09:16:08 PST 2012
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.
-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