[Mesa-dev] [PATCH] mesa/st: emit sampler declarations as arrays when indirect accesses are done
Roland Scheidegger
sroland at vmware.com
Mon Aug 11 10:30:03 PDT 2014
Am 11.08.2014 18:56, schrieb Ilia Mirkin:
> On Mon, Aug 11, 2014 at 12:43 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>> 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?
>
> 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
More information about the mesa-dev
mailing list