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

[Tom Stellard]

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