[Mesa-dev] [PATCH] glsl: atomic counters are different than their uniforms
Timothy Arceri
timothy.arceri at collabora.com
Thu Jun 30 00:55:39 UTC 2016
On Mon, 2016-06-27 at 18:28 +0300, Andres Gomez wrote:
> The linker deals with atomic counters in terms of uniforms. This is
> OK
> but when we want to know the number of used atomic counters since a 2
> elements atomic counters array will use 2 counters but only 1
> uniform.
You don't really mention what you are changing. How about:
glsl: count atomic counters correctly.
Currently the linker uses the uniform count for the total number of
atomic counters. However uniforms don't include the innermost array
dimension in their count, but atomic counters are expected to include
them.
<insert supporting spec quote??>
Fixes GL44-CTS.arrays_of_arrays_gl.AtomicDeclaration
>
> Renamed the data structures used in the linker for disambiguation.
You should really have split this in two. One patch to rename things
and one to change make the actual change.
If you fix up the commit message then you can add:
Reviewed-by: Timothy Arceri <timothy.arceri at collabora.com>
I'd also rather you split this into two patches before pushing but I
can live with it if you don't.
Thanks,
Tim
>
> Fixes GL44-CTS.arrays_of_arrays_gl.AtomicDeclaration
>
> Signed-off-by: Andres Gomez <agomez at igalia.com>
> ---
> src/compiler/glsl/link_atomics.cpp | 83 ++++++++++++++++++++------
> ------------
> 1 file changed, 44 insertions(+), 39 deletions(-)
>
> diff --git a/src/compiler/glsl/link_atomics.cpp
> b/src/compiler/glsl/link_atomics.cpp
> index 277d473..10c3c1a 100644
> --- a/src/compiler/glsl/link_atomics.cpp
> +++ b/src/compiler/glsl/link_atomics.cpp
> @@ -30,9 +30,9 @@
>
> namespace {
> /*
> - * Atomic counter as seen by the program.
> + * Atomic counter uniform as seen by the program.
> */
> - struct active_atomic_counter {
> + struct active_atomic_counter_uniform {
> unsigned uniform_loc;
> ir_variable *var;
> };
> @@ -44,44 +44,44 @@ namespace {
> */
> struct active_atomic_buffer {
> active_atomic_buffer()
> - : counters(0), num_counters(0), stage_references(), size(0)
> + : uniforms(0), num_uniforms(0), stage_counter_references(),
> size(0)
> {}
>
> ~active_atomic_buffer()
> {
> - free(counters);
> + free(uniforms);
> }
>
> void push_back(unsigned uniform_loc, ir_variable *var)
> {
> - active_atomic_counter *new_counters;
> + active_atomic_counter_uniform *new_uniforms;
>
> - new_counters = (active_atomic_counter *)
> - realloc(counters, sizeof(active_atomic_counter) *
> - (num_counters + 1));
> + new_uniforms = (active_atomic_counter_uniform *)
> + realloc(uniforms, sizeof(active_atomic_counter_uniform)
> *
> + (num_uniforms + 1));
>
> - if (new_counters == NULL) {
> + if (new_uniforms == NULL) {
> _mesa_error_no_memory(__func__);
> return;
> }
>
> - counters = new_counters;
> - counters[num_counters].uniform_loc = uniform_loc;
> - counters[num_counters].var = var;
> - num_counters++;
> + uniforms = new_uniforms;
> + uniforms[num_uniforms].uniform_loc = uniform_loc;
> + uniforms[num_uniforms].var = var;
> + num_uniforms++;
> }
>
> - active_atomic_counter *counters;
> - unsigned num_counters;
> - unsigned stage_references[MESA_SHADER_STAGES];
> + active_atomic_counter_uniform *uniforms;
> + unsigned num_uniforms;
> + unsigned stage_counter_references[MESA_SHADER_STAGES];
> unsigned size;
> };
>
> int
> cmp_actives(const void *a, const void *b)
> {
> - const active_atomic_counter *const first =
> (active_atomic_counter *) a;
> - const active_atomic_counter *const second =
> (active_atomic_counter *) b;
> + const active_atomic_counter_uniform *const first =
> (active_atomic_counter_uniform *) a;
> + const active_atomic_counter_uniform *const second =
> (active_atomic_counter_uniform *) b;
>
> return int(first->var->data.offset) - int(second->var-
> >data.offset);
> }
> @@ -103,9 +103,9 @@ namespace {
> const unsigned shader_stage)
> {
> /* FIXME: Arrays of arrays get counted separately. For
> example:
> - * x1[3][3][2] = 9 counters
> - * x2[3][2] = 3 counters
> - * x3[2] = 1 counter
> + * x1[3][3][2] = 9 uniforms, 18 atomic counters
> + * x2[3][2] = 3 uniforms, 6 atomic counters
> + * x3[2] = 1 uniforms, 2 atomic counters
> *
> * However this code marks all the counters as active even
> when they
> * might not be used.
> @@ -129,7 +129,12 @@ namespace {
>
> buf->push_back(*uniform_loc, var);
>
> - buf->stage_references[shader_stage]++;
> + /* When checking for atomic counters we should count every
> member in
> + * an array as an atomic counter reference. */
> + if (t->is_array())
> + buf->stage_counter_references[shader_stage] += t-
> >length;
> + else
> + buf->stage_counter_references[shader_stage]++;
> buf->size = MAX2(buf->size, *offset + t->atomic_size());
>
> storage->offset = *offset;
> @@ -170,22 +175,22 @@ namespace {
> if (buffers[i].size == 0)
> continue;
>
> - qsort(buffers[i].counters, buffers[i].num_counters,
> - sizeof(active_atomic_counter),
> + qsort(buffers[i].uniforms, buffers[i].num_uniforms,
> + sizeof(active_atomic_counter_uniform),
> cmp_actives);
>
> - for (unsigned j = 1; j < buffers[i].num_counters; j++) {
> + for (unsigned j = 1; j < buffers[i].num_uniforms; j++) {
> /* If an overlapping counter found, it must be a
> reference to the
> * same counter from a different shader stage.
> */
> - if (check_atomic_counters_overlap(buffers[i].counters[j-
> 1].var,
> - buffers[i].counters[j]
> .var)
> - && strcmp(buffers[i].counters[j-1].var->name,
> - buffers[i].counters[j].var->name) != 0) {
> + if (check_atomic_counters_overlap(buffers[i].uniforms[j-
> 1].var,
> + buffers[i].uniforms[j]
> .var)
> + && strcmp(buffers[i].uniforms[j-1].var->name,
> + buffers[i].uniforms[j].var->name) != 0) {
> linker_error(prog, "Atomic counter %s declared at
> offset %d "
> "which is already in use.",
> - buffers[i].counters[j].var->name,
> - buffers[i].counters[j].var-
> >data.offset);
> + buffers[i].uniforms[j].var->name,
> + buffers[i].uniforms[j].var-
> >data.offset);
> }
> }
> }
> @@ -223,16 +228,16 @@ link_assign_atomic_counter_resources(struct
> gl_context *ctx,
> mab.Binding = binding;
> mab.MinimumSize = ab.size;
> mab.Uniforms = rzalloc_array(prog->AtomicBuffers, GLuint,
> - ab.num_counters);
> - mab.NumUniforms = ab.num_counters;
> + ab.num_uniforms);
> + mab.NumUniforms = ab.num_uniforms;
>
> /* Assign counter-specific fields. */
> - for (unsigned j = 0; j < ab.num_counters; j++) {
> - ir_variable *const var = ab.counters[j].var;
> + for (unsigned j = 0; j < ab.num_uniforms; j++) {
> + ir_variable *const var = ab.uniforms[j].var;
> gl_uniform_storage *const storage =
> - &prog->UniformStorage[ab.counters[j].uniform_loc];
> + &prog->UniformStorage[ab.uniforms[j].uniform_loc];
>
> - mab.Uniforms[j] = ab.counters[j].uniform_loc;
> + mab.Uniforms[j] = ab.uniforms[j].uniform_loc;
> if (!var->data.explicit_binding)
> var->data.binding = i;
>
> @@ -246,7 +251,7 @@ link_assign_atomic_counter_resources(struct
> gl_context *ctx,
>
> /* Assign stage-specific fields. */
> for (unsigned j = 0; j < MESA_SHADER_STAGES; ++j) {
> - if (ab.stage_references[j]) {
> + if (ab.stage_counter_references[j]) {
> mab.StageReferences[j] = GL_TRUE;
> num_atomic_buffers[j]++;
> } else {
> @@ -314,7 +319,7 @@ link_check_atomic_counter_resources(struct
> gl_context *ctx,
> continue;
>
> for (unsigned j = 0; j < MESA_SHADER_STAGES; ++j) {
> - const unsigned n = abs[i].stage_references[j];
> + const unsigned n = abs[i].stage_counter_references[j];
>
> if (n) {
> atomic_counters[j] += n;
More information about the mesa-dev
mailing list