[Mesa-dev] [PATCH] mesa/st: emit sampler declarations as arrays when indirect accesses are done
Roland Scheidegger
sroland at vmware.com
Mon Aug 11 09:43:33 PDT 2014
Am 11.08.2014 17:18, schrieb Ilia Mirkin:
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>
> This applies on top of my previous patch to allow dynamic sampler offsets.
>
> Roland, I believe this should address your concerns. The generated code looks
> like:
>
> FRAG
> PROPERTY FS_COLOR0_WRITES_ALL_CBUFS 1
> DCL IN[0], GENERIC[0], PERSPECTIVE
> DCL OUT[0], COLOR
> DCL SAMP[0]
> DCL SAMP[1..3]
> DCL CONST[4]
> DCL TEMP[0..1], LOCAL
> DCL ADDR[0..2]
> IMM[0] FLT32 { 0.0000, 0.0000, 0.0000, 0.0000}
> 0: MOV TEMP[0].xy, IN[0].xyyy
> 1: TEX TEMP[0], TEMP[0], SAMP[0], 2D
> 2: MOV TEMP[1].xy, IN[0].xyyy
> 3: UARL ADDR[2].x, CONST[4].xxxx
> 4: TEX TEMP[1], TEMP[1], SAMP[ADDR[2].x+1], 2D
> 5: MAD TEMP[0], TEMP[0], IMM[0].xxxx, TEMP[1]
> 6: MOV OUT[0], TEMP[0]
> 7: END
>
> And for now, the +1 is definitely guaranteed to be the base of the sampler
> array. If some future optimization pass materializes, it will have to update
> this code since it won't work otherwise. On the bright side, it's unlikely
> that it'd poke around in inst->sampler, at least not without fixing things up
> first.
>
> src/gallium/auxiliary/tgsi/tgsi_ureg.c | 48 +++++++++++++++++++++++-------
> src/gallium/auxiliary/tgsi/tgsi_ureg.h | 5 ++++
> src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 16 ++++++----
> 3 files changed, 54 insertions(+), 15 deletions(-)
>
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.c b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
> index 6d3ac91..00c671b 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
> @@ -141,7 +141,10 @@ struct ureg_program
> } immediate[UREG_MAX_IMMEDIATE];
> unsigned nr_immediates;
>
> - struct ureg_src sampler[PIPE_MAX_SAMPLERS];
> + struct {
> + struct ureg_src sampler;
> + unsigned size;
> + } sampler[PIPE_MAX_SAMPLERS];
> unsigned nr_samplers;
>
> struct {
> @@ -663,19 +666,43 @@ struct ureg_src ureg_DECL_sampler( struct ureg_program *ureg,
> unsigned i;
>
> for (i = 0; i < ureg->nr_samplers; i++)
> - if (ureg->sampler[i].Index == nr)
> - return ureg->sampler[i];
> -
> + if (ureg->sampler[i].sampler.Index == nr)
> + return ureg->sampler[i].sampler;
> +
> + if (i < PIPE_MAX_SAMPLERS) {
> + ureg->sampler[i].sampler = ureg_src_register( TGSI_FILE_SAMPLER, nr );
> + ureg->sampler[i].size = 1;
> + ureg->nr_samplers++;
> + return ureg->sampler[i].sampler;
> + }
> +
> + assert( 0 );
> + return ureg->sampler[0].sampler;
> +}
> +
> +struct ureg_src
> +ureg_DECL_sampler_array( struct ureg_program *ureg,
> + unsigned nr,
> + unsigned elements )
> +{
> + unsigned i;
> +
> + for (i = 0; i < ureg->nr_samplers; i++)
> + if (ureg->sampler[i].sampler.Index == nr)
> + return ureg->sampler[i].sampler;
> +
> if (i < PIPE_MAX_SAMPLERS) {
> - ureg->sampler[i] = ureg_src_register( TGSI_FILE_SAMPLER, nr );
> + ureg->sampler[i].sampler = ureg_src_register( TGSI_FILE_SAMPLER, nr );
> + ureg->sampler[i].size = elements;
> ureg->nr_samplers++;
> - return ureg->sampler[i];
> + return ureg->sampler[i].sampler;
> }
>
> assert( 0 );
> - return ureg->sampler[0];
> + return ureg->sampler[0].sampler;
> }
>
> +
> /*
> * Allocate a new shader sampler view.
> */
> @@ -1571,9 +1598,10 @@ static void emit_decls( struct ureg_program *ureg )
> }
>
> for (i = 0; i < ureg->nr_samplers; i++) {
> - emit_decl_range( ureg,
> - TGSI_FILE_SAMPLER,
> - ureg->sampler[i].Index, 1 );
> + emit_decl_range(ureg,
> + TGSI_FILE_SAMPLER,
> + ureg->sampler[i].sampler.Index,
> + ureg->sampler[i].size);
> }
>
> for (i = 0; i < ureg->nr_sampler_views; i++) {
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.h b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
> index f014b53..0512efa 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.h
> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
> @@ -325,6 +325,11 @@ ureg_DECL_sampler( struct ureg_program *,
> unsigned index );
>
> struct ureg_src
> +ureg_DECL_sampler_array( struct ureg_program *,
> + unsigned index,
> + unsigned elements );
> +
> +struct ureg_src
> ureg_DECL_sampler_view(struct ureg_program *,
> unsigned index,
> unsigned target,
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 1fc229c..29b0742 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -31,6 +31,7 @@
> */
>
> #include <stdio.h>
> +#include <map>
> #include "main/compiler.h"
> #include "ir.h"
> #include "ir_visitor.h"
> @@ -334,8 +335,10 @@ public:
> unsigned array_sizes[MAX_ARRAYS];
> unsigned next_array;
>
> - int num_address_regs;
> int samplers_used;
> + std::map<int, int> sampler_array_sizes;
> +
> + int num_address_regs;
> bool indirect_addr_consts;
>
> int glsl_version;
> @@ -3229,9 +3232,12 @@ static void
> count_resources(glsl_to_tgsi_visitor *v, gl_program *prog)
> {
> v->samplers_used = 0;
> + v->sampler_array_sizes.clear();
>
> foreach_in_list(glsl_to_tgsi_instruction, inst, &v->instructions) {
> if (is_tex_instruction(inst->op)) {
> + v->sampler_array_sizes.insert(
> + std::make_pair(inst->sampler.index, inst->sampler_array_size));
> for (int i = 0; i < inst->sampler_array_size; i++) {
> v->samplers_used |= 1 << (inst->sampler.index + i);
>
> @@ -5115,10 +5121,10 @@ st_translate_program(
> assert(i == program->num_immediates);
>
> /* texture samplers */
> - for (i = 0; i < ctx->Const.Program[MESA_SHADER_FRAGMENT].MaxTextureImageUnits; i++) {
> - if (program->samplers_used & (1 << i)) {
> - t->samplers[i] = ureg_DECL_sampler(ureg, i);
> - }
> + for (std::map<int, int>::const_iterator it = program->sampler_array_sizes.begin();
> + it != program->sampler_array_sizes.end(); ++it) {
> + t->samplers[it->first] = ureg_DECL_sampler_array(
> + ureg, it->first, it->second);
> }
>
> /* Emit each instruction in turn:
>
The tgsi generated looks ok to me. I don't think though it can work
correctly because samplers_used may be set by other places?
Also, I suspect there could be a problem (though I didn't look at the
glsl code) due to the way that the array ranges are derived from the
instructions. Wouldn't something like
uniform sampler2D tex[8];
someinttemp i;
x = texture2D(tex[i]...)
y = texture2D(tex[2]...)
get the dcls wrong, so the result would be something like
DCL SAMP[0-7]
DCL SAMP[2]
or worse, if the second texture instruction would address tex[0]
you'd get just
DCL SAMP[0] because it overwrote the first dcl?
Maybe this is more complex than I thought...
(OTOH you can see from the posted intel patches that some hw really
could make use of the array ranges, so this is not just purely a
theoretical issue.)
Roland
More information about the mesa-dev
mailing list