[Mesa-dev] [PATCH V2] glsl: fix atomic buffer index for bindings other than 0

Timothy Arceri t_arceri at yahoo.com.au
Sat Jul 25 07:24:47 PDT 2015


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 we store atomic buffer index in the unused var->data.index
to avoid the extra memory of putting back the atmoic buffer index field.

V2: store buffer index in var->data.index and uniform slot in
var->data.location to avoid issues when linking more than 2 shaders.
Also some small tidy ups.

Cc: Alejandro PiƱeiro <apinheiro at igalia.com>
Cc: Ian Romanick <idr at freedesktop.org>
Cc: 10.4, 10.5 <mesa-stable at lists.freedesktop.org>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90175
---
 src/glsl/ir.h                                  |  3 +++
 src/glsl/link_atomics.cpp                      | 18 +++++++-----------
 src/glsl/link_uniforms.cpp                     |  4 ++++
 src/glsl/nir/glsl_to_nir.cpp                   |  2 --
 src/glsl/nir/nir.h                             |  6 ++++--
 src/glsl/nir/nir_lower_atomics.c               |  2 +-
 src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |  2 +-
 7 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index ede8caa..e76b0ec 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -757,6 +757,8 @@ public:
        * \note
        * The GLSL spec only allows the values 0 or 1 for the index in \b dual
        * source blending.
+       *
+       * For atomic counters this stores the atomic buffer index.
        */
       unsigned index:1;
 
@@ -819,6 +821,7 @@ public:
        *   - Fragment shader output: one of the values from \c gl_frag_result.
        *   - Uniforms: Per-stage uniform slot number for default uniform block.
        *   - Uniforms: Index within the uniform block definition for UBO members.
+       *   - Atomic Counter: Uniform slot number.
        *   - Other: This field is not currently used.
        *
        * If the variable is a uniform, shader input, or shader output, and the
diff --git a/src/glsl/link_atomics.cpp b/src/glsl/link_atomics.cpp
index 100d03c..5d3c40f 100644
--- a/src/glsl/link_atomics.cpp
+++ b/src/glsl/link_atomics.cpp
@@ -33,7 +33,6 @@ namespace {
     * Atomic counter as seen by the program.
     */
    struct active_atomic_counter {
-      unsigned id;
       ir_variable *var;
    };
 
@@ -52,7 +51,7 @@ namespace {
          free(counters);
       }
 
-      void push_back(unsigned id, ir_variable *var)
+      void push_back(ir_variable *var)
       {
          active_atomic_counter *new_counters;
 
@@ -66,7 +65,6 @@ namespace {
          }
 
          counters = new_counters;
-         counters[num_counters].id = id;
          counters[num_counters].var = var;
          num_counters++;
       }
@@ -114,10 +112,6 @@ namespace {
             ir_variable *var = node->as_variable();
 
             if (var && var->type->contains_atomic()) {
-               unsigned id = 0;
-               bool found = prog->UniformHash->get(id, var->name);
-               assert(found);
-               (void) found;
                active_atomic_buffer *buf = &buffers[var->data.binding];
 
                /* If this is the first time the buffer is used, increment
@@ -126,7 +120,7 @@ namespace {
                if (buf->size == 0)
                   (*num_buffers)++;
 
-               buf->push_back(id, var);
+               buf->push_back(var);
 
                buf->stage_references[i]++;
                buf->size = MAX2(buf->size, var->data.atomic.offset +
@@ -197,13 +191,15 @@ link_assign_atomic_counter_resources(struct gl_context *ctx,
       /* Assign counter-specific fields. */
       for (unsigned j = 0; j < ab.num_counters; j++) {
          ir_variable *const var = ab.counters[j].var;
-         const unsigned id = ab.counters[j].id;
-         gl_uniform_storage *const storage = &prog->UniformStorage[id];
+         gl_uniform_storage *const storage =
+            &prog->UniformStorage[var->data.location];
 
-         mab.Uniforms[j] = id;
+         mab.Uniforms[j] = var->data.location;
          if (!var->data.explicit_binding)
             var->data.binding = i;
 
+         var->data.index = i;
+
          storage->atomic_buffer_index = i;
          storage->offset = var->data.atomic.offset;
          storage->array_stride = (var->type->is_array() ?
diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
index 61b47c9..874db09 100644
--- a/src/glsl/link_uniforms.cpp
+++ b/src/glsl/link_uniforms.cpp
@@ -620,6 +620,10 @@ private:
       handle_images(base_type, &this->uniforms[id]);
       handle_subroutines(base_type, &this->uniforms[id]);
 
+      if (base_type->contains_atomic()) {
+         current_var->data.location = id;
+      }
+
       /* If there is already storage associated with this uniform or if the
        * uniform is set as builtin, it means that it was set while processing
        * an earlier shader stage.  For example, we may be processing the
diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
index 66430f3..108222d 100644
--- a/src/glsl/nir/glsl_to_nir.cpp
+++ b/src/glsl/nir/glsl_to_nir.cpp
@@ -326,8 +326,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 62cdbd4..d97db68 100644
--- a/src/glsl/nir/nir.h
+++ b/src/glsl/nir/nir.h
@@ -278,6 +278,7 @@ typedef struct {
        *   - Fragment shader output: one of the values from \c gl_frag_result.
        *   - Uniforms: Per-stage uniform slot number for default uniform block.
        *   - Uniforms: Index within the uniform block definition for UBO members.
+       *   - Atomic Counter: Uniform slot number.
        *   - Other: This field is not currently used.
        *
        * If the variable is a uniform, shader input, or shader output, and the
@@ -292,7 +293,9 @@ typedef struct {
       unsigned int driver_location;
 
       /**
-       * output index for dual source blending.
+       * Output index for dual source blending.
+       *
+       * For atomic counters this stores the atomic buffer index.
        */
       int index;
 
@@ -307,7 +310,6 @@ typedef struct {
        * Location an atomic counter is stored at.
        */
       struct {
-         unsigned buffer_index;
          unsigned offset;
       } atomic;
 
diff --git a/src/glsl/nir/nir_lower_atomics.c b/src/glsl/nir/nir_lower_atomics.c
index ce3615a..93db3a9 100644
--- a/src/glsl/nir/nir_lower_atomics.c
+++ b/src/glsl/nir/nir_lower_atomics.c
@@ -63,7 +63,7 @@ lower_instr(nir_intrinsic_instr *instr, nir_function_impl *impl)
 
    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;
+      (int) instr->variables[0]->var->data.location;
 
    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;
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index a6eee47..5844c7c 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -2400,7 +2400,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);
+                          location->data.index);
 
    /* Calculate the surface offset */
    src_reg offset(this, glsl_type::uint_type);
-- 
2.4.3



More information about the mesa-dev mailing list