Am 11.08.2014 18:56, schrieb Ilia Mirkin: > On Mon, Aug 11, 2014 at 12:43 PM, Roland Scheidegger <srol...@vmware.com> > wrote: >> Am 11.08.2014 17:18, schrieb Ilia Mirkin: >>> Signed-off-by: Ilia Mirkin <imir...@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? > > Pretty sure samplers_used can just be dropped entirely. All the places > that set it also generate instructions that use the samplers. There's > no way to "implicitly" use a sampler without a tex instruction of some > sort, AFAIK. > >> 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? > > Right, that's no good. Thanks for catching that :) I'll need to > rethink the impl... ugh. > >> >> 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.) > > Do you think my first patch can go in without this one being ready? > There's a release on the 15th, and I was hoping to make that with > ARB_gs5 on nvc0. >
I'm ok with that. Roland _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev