[Mesa-dev] [RFC 1/5] nir: Separate texture from sampler in nir_tex_instr

Jason Ekstrand jason at jlekstrand.net
Wed Nov 25 18:56:06 PST 2015


On Wed, Nov 25, 2015 at 3:57 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On Tuesday, November 03, 2015 01:26:00 PM Jason Ekstrand wrote:
>> This commit adds the capability to NIR to support separate textures and
>> samplers.  As it currently stands, glsl_to_nir only sets the sampler and
>> leaves the texture alone as it did before and nir_lower_samplers assumes
>> this.  However, backends can, if they wish, assume that they are separate
>> because nir_lower_samplers sets both texture and sampler index (they are
>> the same in this case).
>> ---
>>  src/glsl/nir/nir.c                         |  8 +++++++-
>>  src/glsl/nir/nir.h                         | 28 ++++++++++++++++++++++++----
>>  src/glsl/nir/nir_instr_set.c               | 13 ++++++++-----
>>  src/glsl/nir/nir_lower_samplers.c          | 15 +++++++++++++--
>>  src/glsl/nir/nir_print.c                   | 14 +++++++++++---
>>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp   |  2 +-
>>  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp |  2 +-
>>  src/mesa/program/prog_to_nir.c             |  1 +
>>  8 files changed, 66 insertions(+), 17 deletions(-)
>>
>> diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
>> index 5f03095..d7f5909 100644
>> --- a/src/glsl/nir/nir.c
>> +++ b/src/glsl/nir/nir.c
>> @@ -488,8 +488,10 @@ nir_tex_instr_create(nir_shader *shader, unsigned num_srcs)
>>     for (unsigned i = 0; i < num_srcs; i++)
>>        src_init(&instr->src[i].src);
>>
>> +   instr->texture_index = 0;
>> +   instr->texture_array_size = 0;
>> +   instr->texture = NULL;
>>     instr->sampler_index = 0;
>> -   instr->sampler_array_size = 0;
>>     instr->sampler = NULL;
>>
>>     return instr;
>> @@ -1007,6 +1009,10 @@ visit_tex_src(nir_tex_instr *instr, nir_foreach_src_cb cb, void *state)
>>        if (!visit_src(&instr->src[i].src, cb, state))
>>           return false;
>>
>> +   if (instr->texture != NULL)
>
> Mesa coding style is to use braces here, as the contents of the if span
> more than a single line (even if it is technically a single statement).
>
>> +      if (!visit_deref_src(instr->texture, cb, state))
>> +         return false;
>> +
>>     if (instr->sampler != NULL)
>>        if (!visit_deref_src(instr->sampler, cb, state))
>>           return false;
>> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
>> index ac42251..682c4ed 100644
>> --- a/src/glsl/nir/nir.h
>> +++ b/src/glsl/nir/nir.h
>> @@ -933,6 +933,7 @@ typedef enum {
>>     nir_tex_src_ms_index, /* MSAA sample index */
>>     nir_tex_src_ddx,
>>     nir_tex_src_ddy,
>> +   nir_tex_src_texture_offset, /* < dynamically uniform indirect offset */
>>     nir_tex_src_sampler_offset, /* < dynamically uniform indirect offset */
>>     nir_num_tex_src_types
>>  } nir_tex_src_type;
>> @@ -980,6 +981,24 @@ typedef struct {
>>     /* gather component selector */
>>     unsigned component : 2;
>>
>> +   /** The texture index
>> +    *
>> +    * If this texture instruction has a nir_tex_src_texture_offset source,
>> +    * then the texture index is given by texture_index + texture_offset.
>> +    */
>> +   unsigned texture_index;
>> +
>> +   /** The size of the texture array or 0 if it's not an array */
>> +   unsigned texture_array_size;
>> +
>> +   /** The texture deref
>> +    *
>> +    * If both this and `sampler` are both NULL, use texture_index instead.
>> +    * If `texture` is NULL, but `sampler` is non-NULL, then the texture is
>> +    * implied from the sampler.
>> +    */
>> +   nir_deref_var *texture;
>> +
>>     /** The sampler index
>>      *
>>      * If this texture instruction has a nir_tex_src_sampler_offset source,
>> @@ -987,10 +1006,11 @@ typedef struct {
>>      */
>>     unsigned sampler_index;
>>
>> -   /** The size of the sampler array or 0 if it's not an array */
>> -   unsigned sampler_array_size;
>> -
>> -   nir_deref_var *sampler; /* if this is NULL, use sampler_index instead */
>> +   /** The sampler deref
>> +    *
>> +    * If this is null, use sampler_index instead.
>> +    */
>> +   nir_deref_var *sampler;
>>  } nir_tex_instr;
>>
>>  static inline unsigned
>> diff --git a/src/glsl/nir/nir_instr_set.c b/src/glsl/nir/nir_instr_set.c
>> index d3f939f..eb02132 100644
>> --- a/src/glsl/nir/nir_instr_set.c
>> +++ b/src/glsl/nir/nir_instr_set.c
>> @@ -155,8 +155,9 @@ hash_tex(uint32_t hash, const nir_tex_instr *instr)
>>     hash = HASH(hash, instr->const_offset);
>>     unsigned component = instr->component;
>>     hash = HASH(hash, component);
>> +   hash = HASH(hash, instr->texture_index);
>> +   hash = HASH(hash, instr->texture_array_size);
>>     hash = HASH(hash, instr->sampler_index);
>> -   hash = HASH(hash, instr->sampler_array_size);
>>
>>     assert(!instr->sampler);
>>
>> @@ -305,13 +306,15 @@ nir_instrs_equal(const nir_instr *instr1, const nir_instr *instr2)
>>            memcmp(tex1->const_offset, tex2->const_offset,
>>                   sizeof(tex1->const_offset)) != 0 ||
>>            tex1->component != tex2->component ||
>> -         tex1->sampler_index != tex2->sampler_index ||
>> -         tex1->sampler_array_size != tex2->sampler_array_size) {
>> +         tex1->texture_index != tex2->texture_index ||
>> +         tex1->texture_array_size != tex2->texture_array_size ||
>> +         tex1->sampler_index != tex2->sampler_index) {
>>           return false;
>>        }
>>
>>        /* Don't support un-lowered sampler derefs currently. */
>> -      assert(!tex1->sampler && !tex2->sampler);
>> +      assert(!tex1->texture && !tex1->sampler &&
>> +             !tex2->texture && !tex2->sampler);
>>
>>        return true;
>>     }
>> @@ -422,7 +425,7 @@ instr_can_rewrite(nir_instr *instr)
>>        nir_tex_instr *tex = nir_instr_as_tex(instr);
>>
>>        /* Don't support un-lowered sampler derefs currently. */
>> -      if (tex->sampler)
>> +      if (tex->texture || tex->sampler)
>>           return false;
>>
>>        return true;
>> diff --git a/src/glsl/nir/nir_lower_samplers.c b/src/glsl/nir/nir_lower_samplers.c
>> index 5df79a6..19deafa 100644
>> --- a/src/glsl/nir/nir_lower_samplers.c
>> +++ b/src/glsl/nir/nir_lower_samplers.c
>> @@ -95,6 +95,9 @@ lower_sampler(nir_tex_instr *instr, const struct gl_shader_program *shader_progr
>>     if (instr->sampler == NULL)
>>        return;
>>
>> +   /* GLSL only has combined textures/samplers */
>> +   assert(instr->texture == NULL);
>> +
>>     instr->sampler_index = 0;
>>     unsigned location = instr->sampler->var->data.location;
>>     unsigned array_elements = 1;
>> @@ -107,7 +110,7 @@ lower_sampler(nir_tex_instr *instr, const struct gl_shader_program *shader_progr
>>     if (indirect) {
>>        /* First, we have to resize the array of texture sources */
>>        nir_tex_src *new_srcs = rzalloc_array(instr, nir_tex_src,
>> -                                            instr->num_srcs + 1);
>> +                                            instr->num_srcs + 2);
>>
>>        for (unsigned i = 0; i < instr->num_srcs; i++) {
>>           new_srcs[i].src_type = instr->src[i].src_type;
>> @@ -121,13 +124,19 @@ lower_sampler(nir_tex_instr *instr, const struct gl_shader_program *shader_progr
>>        /* Now we can go ahead and move the source over to being a
>>         * first-class texture source.
>>         */
>> +      instr->src[instr->num_srcs].src_type = nir_tex_src_texture_offset;
>> +      instr->num_srcs++;
>> +      nir_instr_rewrite_src(&instr->instr,
>> +                            &instr->src[instr->num_srcs - 1].src,
>> +                            nir_src_for_ssa(indirect));
>> +
>>        instr->src[instr->num_srcs].src_type = nir_tex_src_sampler_offset;
>>        instr->num_srcs++;
>>        nir_instr_rewrite_src(&instr->instr,
>>                              &instr->src[instr->num_srcs - 1].src,
>>                              nir_src_for_ssa(indirect));
>>
>> -      instr->sampler_array_size = array_elements;
>> +      instr->texture_array_size = array_elements;
>>     }
>>
>>     if (location > shader_program->NumUniformStorage - 1 ||
>> @@ -140,6 +149,8 @@ lower_sampler(nir_tex_instr *instr, const struct gl_shader_program *shader_progr
>>        shader_program->UniformStorage[location].opaque[stage].index;
>>
>>     instr->sampler = NULL;
>> +
>> +   instr->texture_index = instr->sampler_index;
>>  }
>>
>>  typedef struct {
>> diff --git a/src/glsl/nir/nir_print.c b/src/glsl/nir/nir_print.c
>> index 30220c5..d38ca11 100644
>> --- a/src/glsl/nir/nir_print.c
>> +++ b/src/glsl/nir/nir_print.c
>> @@ -547,6 +547,9 @@ print_tex_instr(nir_tex_instr *instr, print_state *state)
>>        case nir_tex_src_ddy:
>>           fprintf(fp, "(ddy)");
>>           break;
>> +      case nir_tex_src_texture_offset:
>> +         fprintf(fp, "(texture_offset)");
>> +         break;
>>        case nir_tex_src_sampler_offset:
>>           fprintf(fp, "(sampler_offset)");
>>           break;
>> @@ -577,13 +580,18 @@ print_tex_instr(nir_tex_instr *instr, print_state *state)
>>        fprintf(fp, "%u (gather_component), ", instr->component);
>>     }
>>
>> +   if (instr->texture) {
>> +      assert(instr->sampler);
>
> Seems like this won't work for the SPIR-V case of texelFetch with no
> sampler attached.  Presumably you delete it in your SPIR-V branch?
> Maybe just delete it now?

Yeah.  I must not be printing shaders with null textures before.  I'll
get that cleaned up.

>> +      fprintf(fp, " (texture)");
>> +   }
>>     if (instr->sampler) {
>>        print_deref(instr->sampler, state);
>> +      fprintf(fp, " (sampler)");
>>     } else {
>> -      fprintf(fp, "%u", instr->sampler_index);
>> +      assert(instr->texture == NULL);
>> +      fprintf(fp, "%u (texture) %u (sampler)",
>> +              instr->texture_index, instr->sampler_index);
>>     }
>> -
>> -   fprintf(fp, " (sampler)");
>>  }
>>
>>  static void
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> index 24ff5af..81cabaf 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> @@ -2054,7 +2054,7 @@ fs_visitor::nir_emit_texture(const fs_builder &bld, nir_tex_instr *instr)
>>
>>        case nir_tex_src_sampler_offset: {
>>           /* Figure out the highest possible sampler index and mark it as used */
>> -         uint32_t max_used = sampler + instr->sampler_array_size - 1;
>> +         uint32_t max_used = sampler + instr->texture_array_size - 1;
>>           if (instr->op == nir_texop_tg4 && devinfo->gen < 8) {
>>              max_used += stage_prog_data->binding_table.gather_texture_start;
>>           } else {
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> index 6948a1b..7a6372b 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> @@ -1684,7 +1684,7 @@ vec4_visitor::nir_emit_texture(nir_tex_instr *instr)
>>            * the last element of the array. Mark it here, because the generator
>>            * doesn't have enough information to determine the bound.
>>            */
>> -         uint32_t array_size = instr->sampler_array_size;
>> +         uint32_t array_size = instr->texture_array_size;
>>           uint32_t max_used = sampler + array_size - 1;
>>           if (instr->op == nir_texop_tg4) {
>>              max_used += prog_data->base.binding_table.gather_texture_start;
>> diff --git a/src/mesa/program/prog_to_nir.c b/src/mesa/program/prog_to_nir.c
>> index 539e3c0..d5386ee 100644
>> --- a/src/mesa/program/prog_to_nir.c
>> +++ b/src/mesa/program/prog_to_nir.c
>> @@ -609,6 +609,7 @@ ptn_tex(nir_builder *b, nir_alu_dest dest, nir_ssa_def **src,
>>     instr->op = op;
>>     instr->dest_type = nir_type_float;
>>     instr->is_shadow = prog_inst->TexShadow;
>> +   instr->texture_index = prog_inst->TexSrcUnit;
>>     instr->sampler_index = prog_inst->TexSrcUnit;
>>
>>     switch (prog_inst->TexSrcTarget) {
>>


More information about the mesa-dev mailing list