[Mesa-dev] [PATCH 1/2] tgsi: texture types

[Rob Clark]

On Mon, Jun 8, 2015 at 8:35 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> Am 08.06.2015 um 20:15 schrieb Rob Clark:
>> From: Rob Clark <robclark at freedesktop.org>
>>
>> Freedreno needs sampler type information to deal with int/uint textures.
>> To accomplish this, start creating sampler-view declarations, as
>> suggested here:
>>
>>  http://lists.freedesktop.org/archives/mesa-dev/2014-November/071583.html
>>
>> create a sampler-view with index matching the sampler, to encode the
>> texture type (ie. SINT/UINT/FLOAT).  Ie:
>>
>>    DCL SVIEW[n], 2D, UINT
>>    DCL SAMP[n]
>>    TEX OUT[1], IN[1], SAMP[n]
>>
>> For tgsi texture instructions which do not take an explicit SVIEW
>> argument, the SVIEW index is implied by the SAMP index.
> I wonder if there should be some doc update somewhere.
>

yeah, perhaps.. I wasn't quite sure where, tgsi.rst only talks about
SVIEW in terms of the SAMPLE and related opc's (ie. the ones which do
take an SVIEW arg), and the way things are with this patch, other
drivers simply need to ignore the extra SVIEW decl's and not randomly
assert for things to continue working as before..

hypothetically if something in tree actually created SVIEW decl
currently, and the shader also had TEX* style instructions, it would
have to take care to not have conflicting SVIEW's.  But afaict nothing
in-tree creates sampler view decl's currently.

>>
>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>> ---
>>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 29 +++++++++++++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>>
>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> index 0e60d95..ce8f2ea 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> @@ -239,6 +239,7 @@ public:
>>     st_src_reg sampler; /**< sampler register */
>>     int sampler_array_size; /**< 1-based size of sampler array, 1 if not array */
>>     int tex_target; /**< One of TEXTURE_*_INDEX */
>> +   glsl_base_type tex_type;
>>     GLboolean tex_shadow;
>>
>>     st_src_reg tex_offsets[MAX_GLSL_TEXTURE_OFFSET];
>> @@ -345,6 +346,8 @@ public:
>>
>>     int num_address_regs;
>>     int samplers_used;
>> +   glsl_base_type sampler_types[PIPE_MAX_SAMPLERS];
>> +   int sampler_targets[PIPE_MAX_SAMPLERS];   /**< One of TGSI_TEXTURE_* */
>>     bool indirect_addr_consts;
>>     int wpos_transform_const;
>>
>> @@ -3323,6 +3326,8 @@ glsl_to_tgsi_visitor::visit(ir_texture *ir)
>>        assert(!"Should not get here.");
>>     }
>>
>> +   inst->tex_type = ir->type->base_type;
>> +
>>     this->result = result_src;
>>  }
>>
>> @@ -3471,6 +3476,11 @@ count_resources(glsl_to_tgsi_visitor *v, gl_program *prog)
>>           for (int i = 0; i < inst->sampler_array_size; i++) {
>>              v->samplers_used |= 1 << (inst->sampler.index + i);
>>
>> +            debug_assert(i < (int)ARRAY_SIZE(v->sampler_types));
>> +            v->sampler_types[i] = inst->tex_type;
>> +            v->sampler_targets[i] =
>> +               st_translate_texture_target(inst->tex_target, inst->tex_shadow);
>> +
>>              if (inst->tex_shadow) {
>>                 prog->ShadowSamplers |= 1 << (inst->sampler.index + i);
>>              }
>> @@ -5529,7 +5539,26 @@ st_translate_program(
>>     /* texture samplers */
>>     for (i = 0; i < ctx->Const.Program[MESA_SHADER_FRAGMENT].MaxTextureImageUnits; i++) {
>>        if (program->samplers_used & (1 << i)) {
>> +         unsigned type;
>> +
>>           t->samplers[i] = ureg_DECL_sampler(ureg, i);
>> +
>> +         switch (program->sampler_types[i]) {
>> +         case GLSL_TYPE_INT:
>> +            type = TGSI_RETURN_TYPE_SINT;
>> +            break;
>> +         case GLSL_TYPE_UINT:
>> +            type = TGSI_RETURN_TYPE_UINT;
>> +            break;
>> +         case GLSL_TYPE_FLOAT:
>> +            type = TGSI_RETURN_TYPE_FLOAT;
>> +            break;
>> +         default:
>> +            unreachable("not reached");
>> +         }
>> +
>> +         ureg_DECL_sampler_view( ureg, i, program->sampler_targets[i],
>> +                                 type, type, type, type );
>>        }
>>     }
>>
>>
>
> This indeed seems like a consistent solution. Hopefully drivers don't
> assert when they see sampler view dcls...

well, tgsi_to_nir did before I fixed it :-(

(mostly just because of liberal debug_assert() usage)

it's at least not something I'd land right before a release branch
point.  But I guess the fix is easy enough (ie. remove a bogus
assert), and 10.7 branch point is quite some time from now..

> I am also somewhat worried that this only changes the glsl-to-tgsi path,
> not other paths. llvmpipe and draw rely on either having sview
> declarations for all (sample) instructions or for none, and with draw
> possibly inserting tex opcodes (for aalines etc.) on its own this could
> probably break (these draw paths aren't used with d3d10 sample opcodes
> as it's more or less illegal to use tex and sample opcodes in the same
> shader, but that doesn't really matter here). I think using sview
> declarations is the right thing to do but it probably should be illegal
> then to NOT have them in some places.

fwiw, the way I implemented it in tgsi_to_nir (and what I'd recommend
for other drivers) is that for the "tex style" opcodes, if there is
not a matching SVIEW decl, assume float.  That seemed more sane than
fixing up *all* the other state trackers..

BR,
-R

> Roland
>
>