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

Timothy Arceri t_arceri at yahoo.com.au
Fri Jul 24 21:53:43 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.location
to avoid the extra memory of putting back the atmoic buffer index field.

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
---
 I'm working towards getting rid of UniformHash althogether so this is a nice
 first step.

 The reason for getting rid of UniformHash iss its causing problems for being
 able to use indirect indexing on sampler inside structs this will only get
 worse with arrays of arrays.

 Also I guess it should be a memory win to get rid of the hash table.

 src/glsl/ir.h                                  |  1 +
 src/glsl/link_atomics.cpp                      | 11 ++++++-----
 src/glsl/link_uniforms.cpp                     |  7 +++++++
 src/glsl/nir/glsl_to_nir.cpp                   |  2 --
 src/glsl/nir/nir.h                             |  1 +
 src/glsl/nir/nir_lower_atomics.c               |  2 +-
 src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |  2 +-
 7 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index ede8caa..82e410a 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -819,6 +819,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: Atomic buffer index.
        *   - 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..291b4db 100644
--- a/src/glsl/link_atomics.cpp
+++ b/src/glsl/link_atomics.cpp
@@ -114,10 +114,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 +122,7 @@ namespace {
                if (buf->size == 0)
                   (*num_buffers)++;
 
-               buf->push_back(id, var);
+               buf->push_back(var->data.location, var);
 
                buf->stage_references[i]++;
                buf->size = MAX2(buf->size, var->data.atomic.offset +
@@ -204,6 +200,11 @@ link_assign_atomic_counter_resources(struct gl_context *ctx,
          if (!var->data.explicit_binding)
             var->data.binding = i;
 
+         /* Update var->data.location to the atomic_buffer_index rather than
+          * the uniform location.
+          */
+         var->data.location = 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..3bb579f 100644
--- a/src/glsl/link_uniforms.cpp
+++ b/src/glsl/link_uniforms.cpp
@@ -620,6 +620,13 @@ private:
       handle_images(base_type, &this->uniforms[id]);
       handle_subroutines(base_type, &this->uniforms[id]);
 
+      if (type->contains_atomic()) {
+         /* Temporarily store the uniform location for the atomic, we will
+          * later replace this with the atomic buffer index.
+          */
+         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..6190d6b 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: Atomic buffer index.
        *   - 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/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..2110b62 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.location);
 
    /* Calculate the surface offset */
    src_reg offset(this, glsl_type::uint_type);
-- 
2.4.3



More information about the mesa-stable mailing list