[Mesa-dev] [PATCH 1/4] glsl_to_tgsi: declare all SSBOs and atomics when indirect indexing is used

[Samuel Pitoiset]

On 05/17/2017 09:14 PM, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
> 
> Only the first array element was declared, so tgsi_shader_info::
> shader_buffers_declared didn't match what the shader was using.
> ---
>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 25 +++++++++++++------------
>   1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 54bc70f..e4d139e 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -413,21 +413,20 @@ public:
>      unsigned num_inputs;
>      unsigned num_input_arrays;
>      struct inout_decl outputs[4 * PIPE_MAX_SHADER_OUTPUTS];
>      unsigned num_outputs;
>      unsigned num_output_arrays;
>   
>      int num_address_regs;
>      uint32_t samplers_used;
>      glsl_base_type sampler_types[PIPE_MAX_SAMPLERS];
>      int sampler_targets[PIPE_MAX_SAMPLERS];   /**< One of TGSI_TEXTURE_* */
> -   int buffers_used;
>      int images_used;
>      int image_targets[PIPE_MAX_SHADER_IMAGES];
>      unsigned image_formats[PIPE_MAX_SHADER_IMAGES];
>      bool indirect_addr_consts;
>      int wpos_transform_const;
>   
>      int glsl_version;
>      bool native_integers;
>      bool have_sqrt;
>      bool have_fma;
> @@ -4536,21 +4535,20 @@ glsl_to_tgsi_visitor::glsl_to_tgsi_visitor()
>      array_sizes = NULL;
>      max_num_arrays = 0;
>      next_array = 0;
>      num_inputs = 0;
>      num_outputs = 0;
>      num_input_arrays = 0;
>      num_output_arrays = 0;
>      num_immediates = 0;
>      num_address_regs = 0;
>      samplers_used = 0;
> -   buffers_used = 0;
>      images_used = 0;
>      indirect_addr_consts = false;
>      wpos_transform_const = -1;
>      glsl_version = 0;
>      native_integers = false;
>      mem_ctx = ralloc_context(NULL);
>      ctx = NULL;
>      prog = NULL;
>      shader_program = NULL;
>      shader = NULL;
> @@ -4574,21 +4572,20 @@ extern "C" void free_glsl_to_tgsi_visitor(glsl_to_tgsi_visitor *v)
>   
>   
>   /**
>    * Count resources used by the given gpu program (number of texture
>    * samplers, etc).
>    */
>   static void
>   count_resources(glsl_to_tgsi_visitor *v, gl_program *prog)
>   {
>      v->samplers_used = 0;
> -   v->buffers_used = 0;
>      v->images_used = 0;
>   
>      foreach_in_list(glsl_to_tgsi_instruction, inst, &v->instructions) {
>         if (inst->info->is_tex) {
>            for (int i = 0; i < inst->sampler_array_size; i++) {
>               unsigned idx = inst->sampler_base + i;
>               v->samplers_used |= 1u << idx;
>   
>               debug_assert(idx < (int)ARRAY_SIZE(v->sampler_types));
>               v->sampler_types[idx] = inst->tex_type;
> @@ -4601,21 +4598,21 @@ count_resources(glsl_to_tgsi_visitor *v, gl_program *prog)
>            }
>         }
>   
>         if (inst->tex_target == TEXTURE_EXTERNAL_INDEX)
>            prog->ExternalSamplersUsed |= 1 << inst->resource.index;
>   
>         if (inst->resource.file != PROGRAM_UNDEFINED && (
>                   is_resource_instruction(inst->op) ||
>                   inst->op == TGSI_OPCODE_STORE)) {
>            if (inst->resource.file == PROGRAM_BUFFER) {
> -            v->buffers_used |= 1 << inst->resource.index;
> +            /* nothing to do */

I would suggest to remove the whole 'if' and replace 'else' by 'else if' 
(for images).

>            } else if (inst->resource.file == PROGRAM_MEMORY) {
>               v->use_shared_memory = true;
>            } else {
>               assert(inst->resource.file == PROGRAM_IMAGE);
>               for (int i = 0; i < inst->sampler_array_size; i++) {
>                  unsigned idx = inst->sampler_base + i;
>                  v->images_used |= 1 << idx;
>                  v->image_targets[idx] =
>                     st_translate_texture_target(inst->tex_target, false);
>                  v->image_formats[idx] = inst->image_format;
> @@ -6586,30 +6583,34 @@ st_translate_program(
>               break;
>            default:
>               unreachable("not reached");
>            }
>   
>            ureg_DECL_sampler_view( ureg, i, program->sampler_targets[i],
>                                    type, type, type, type );
>         }
>      }
>   
> -   for (i = 0; i < frag_const->MaxAtomicBuffers; i++) {
> -      if (program->buffers_used & (1 << i)) {
> -         t->buffers[i] = ureg_DECL_buffer(ureg, i, true);
> +   /* Declare atomic and shader storage buffers. */
> +   {
> +      struct gl_program *prog = program->prog;
> +
> +      for (i = 0; i < prog->info.num_abos; i++) {
> +         unsigned index = prog->sh.AtomicBuffers[i]->Binding;
> +         assert(index < frag_const->MaxAtomicBuffers);
> +         t->buffers[index] = ureg_DECL_buffer(ureg, index, true);
>         }
> -   }
>   
> -   for (; i < frag_const->MaxAtomicBuffers + frag_const->MaxShaderStorageBlocks;
> -        i++) {
> -      if (program->buffers_used & (1 << i)) {
> -         t->buffers[i] = ureg_DECL_buffer(ureg, i, false);
> +      assert(prog->info.num_ssbos <= frag_const->MaxShaderStorageBlocks);
> +      for (i = 0; i < prog->info.num_ssbos; i++) {
> +         unsigned index = frag_const->MaxAtomicBuffers + i;
> +         t->buffers[index] = ureg_DECL_buffer(ureg, index, false);
>         }
>      }
>   
>      if (program->use_shared_memory)
>         t->shared_memory = ureg_DECL_memory(ureg, TGSI_MEMORY_TYPE_SHARED);
>   
>      for (i = 0; i < program->shader->Program->info.num_images; i++) {
>         if (program->images_used & (1 << i)) {
>            t->images[i] = ureg_DECL_image(ureg, i,
>                                           program->image_targets[i],
>