[Mesa-dev] [PATCH 14/24] i965: fix atomic buffer index for bindings other than 0

Timothy Arceri t_arceri at yahoo.com.au
Sat Sep 26 14:17:35 PDT 2015



On 27 September 2015 6:23:42 am AEST, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>On Thu, Sep 17, 2015 at 3:02 AM, Timothy Arceri <t_arceri at 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?

It wad added recently [1] as part of my struct indirect indexing fix.

[1] http://cgit.freedesktop.org/mesa/mesa/tree/src/glsl/link_uniforms.cpp#n750


>
>>
>> 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 <apinheiro at 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev



More information about the mesa-dev mailing list