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