On Thu, Sep 17, 2015 at 3:02 AM, Timothy Arceri <t_arc...@yahoo.com.au> wrote:
> Since commit c0cd5b var->data.binding was being used as a replacement
> for atomic buffer index, but they don't have to be the same value they
> just happen to end up the same when binding is 0.
>
> Now that we store the atomic uniform location in var->data.location
> we can use this to lookup the atomic buffer index in uniform storage.

FWIW I ran into a similar problem when trying to implement atomic
counters in gallium. I assume that this logic depends on some other
bit from this patch series, since I don't see that we store the value
in var->data.location in the upstream code?

>
> For arrays of arrays the outer arrays have separate uniform locations
> however they all share the same buffer so we can get the buffer index
> using the base uniform location.
>
> Cc: Alejandro Piñeiro <apinhe...@igalia.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90175
> ---
>  src/glsl/nir/glsl_to_nir.cpp                   |  2 --
>  src/glsl/nir/nir.h                             |  4 ++--
>  src/glsl/nir/nir_lower_atomics.c               | 18 ++++++++++++------
>  src/mesa/drivers/dri/i965/brw_nir.c            |  6 ++++--
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |  2 +-
>  5 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
> index c13f953..6b2da89 100644
> --- a/src/glsl/nir/glsl_to_nir.cpp
> +++ b/src/glsl/nir/glsl_to_nir.cpp
> @@ -330,8 +330,6 @@ nir_visitor::visit(ir_variable *ir)
>
>     var->data.index = ir->data.index;
>     var->data.binding = ir->data.binding;
> -   /* XXX Get rid of buffer_index */
> -   var->data.atomic.buffer_index = ir->data.binding;
>     var->data.atomic.offset = ir->data.atomic.offset;
>     var->data.image.read_only = ir->data.image_read_only;
>     var->data.image.write_only = ir->data.image_write_only;
> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
> index 3a19bd3..a974188 100644
> --- a/src/glsl/nir/nir.h
> +++ b/src/glsl/nir/nir.h
> @@ -308,7 +308,6 @@ typedef struct {
>         * Location an atomic counter is stored at.
>         */
>        struct {
> -         unsigned buffer_index;
>           unsigned offset;
>        } atomic;
>
> @@ -1834,7 +1833,8 @@ void nir_lower_system_values(nir_shader *shader);
>  void nir_lower_tex_projector(nir_shader *shader);
>  void nir_lower_idiv(nir_shader *shader);
>
> -void nir_lower_atomics(nir_shader *shader);
> +void nir_lower_atomics(nir_shader *shader,
> +                       const struct gl_shader_program *shader_program);
>  void nir_lower_to_source_mods(nir_shader *shader);
>
>  void nir_normalize_cubemap_coords(nir_shader *shader);
> diff --git a/src/glsl/nir/nir_lower_atomics.c 
> b/src/glsl/nir/nir_lower_atomics.c
> index 46e1376..52e7675 100644
> --- a/src/glsl/nir/nir_lower_atomics.c
> +++ b/src/glsl/nir/nir_lower_atomics.c
> @@ -25,6 +25,7 @@
>   *
>   */
>
> +#include "ir_uniform.h"
>  #include "nir.h"
>  #include "main/config.h"
>  #include <assert.h>
> @@ -35,7 +36,8 @@
>   */
>
>  static void
> -lower_instr(nir_intrinsic_instr *instr, nir_function_impl *impl)
> +lower_instr(nir_intrinsic_instr *instr,
> +            const struct gl_shader_program *shader_program)
>  {
>     nir_intrinsic_op op;
>     switch (instr->intrinsic) {
> @@ -60,10 +62,11 @@ lower_instr(nir_intrinsic_instr *instr, nir_function_impl 
> *impl)
>        return; /* atomics passed as function arguments can't be lowered */
>
>     void *mem_ctx = ralloc_parent(instr);
> +   unsigned uniform_loc = instr->variables[0]->var->data.location;
>
>     nir_intrinsic_instr *new_instr = nir_intrinsic_instr_create(mem_ctx, op);
>     new_instr->const_index[0] =
> -      (int) instr->variables[0]->var->data.atomic.buffer_index;
> +      shader_program->UniformStorage[uniform_loc].atomic_buffer_index;
>
>     nir_load_const_instr *offset_const = nir_load_const_instr_create(mem_ctx, 
> 1);
>     offset_const->value.u[0] = instr->variables[0]->var->data.atomic.offset;
> @@ -128,22 +131,25 @@ lower_instr(nir_intrinsic_instr *instr, 
> nir_function_impl *impl)
>  }
>
>  static bool
> -lower_block(nir_block *block, void *state)
> +lower_block(nir_block *block, void *prog)
>  {
>     nir_foreach_instr_safe(block, instr) {
>        if (instr->type == nir_instr_type_intrinsic)
> -         lower_instr(nir_instr_as_intrinsic(instr), state);
> +         lower_instr(nir_instr_as_intrinsic(instr),
> +                     (const struct gl_shader_program *) prog);
>     }
>
>     return true;
>  }
>
>  void
> -nir_lower_atomics(nir_shader *shader)
> +nir_lower_atomics(nir_shader *shader,
> +                  const struct gl_shader_program *shader_program)
>  {
>     nir_foreach_overload(shader, overload) {
>        if (overload->impl) {
> -         nir_foreach_block(overload->impl, lower_block, overload->impl);
> +         nir_foreach_block(overload->impl, lower_block,
> +                           (void *) shader_program);
>           nir_metadata_preserve(overload->impl, nir_metadata_block_index |
>                                                 nir_metadata_dominance);
>        }
> diff --git a/src/mesa/drivers/dri/i965/brw_nir.c 
> b/src/mesa/drivers/dri/i965/brw_nir.c
> index f326b23..fc7dc1e 100644
> --- a/src/mesa/drivers/dri/i965/brw_nir.c
> +++ b/src/mesa/drivers/dri/i965/brw_nir.c
> @@ -147,8 +147,10 @@ brw_create_nir(struct brw_context *brw,
>     nir_lower_system_values(nir);
>     nir_validate_shader(nir);
>
> -   nir_lower_atomics(nir);
> -   nir_validate_shader(nir);
> +   if (shader_prog) {
> +      nir_lower_atomics(nir, shader_prog);
> +      nir_validate_shader(nir);
> +   }
>
>     nir_optimize(nir, is_scalar);
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 733d5ad..f818d78 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -2424,7 +2424,7 @@ vec4_visitor::visit_atomic_counter_intrinsic(ir_call 
> *ir)
>        ir->actual_parameters.get_head());
>     ir_variable *location = deref->variable_referenced();
>     unsigned surf_index = (prog_data->base.binding_table.abo_start +
> -                          location->data.binding);
> +      
> shader_prog->UniformStorage[location->data.location].atomic_buffer_index);
>
>     /* Calculate the surface offset */
>     src_reg offset(this, glsl_type::uint_type);
> --
> 2.4.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to