On 23/10/15 12:11, Timothy Arceri wrote: > On Fri, 2015-10-23 at 08:57 +0200, Samuel Iglesias Gonsalvez wrote: >> Atomic counter variables can have a 'binding' layout modifier. >> Unfortunately, >> the atomic counter buffers are not sorted by binding value in >> gl_shader_program's AtomicBuffers. >> >> Save the atomic counter buffer block index into the variable, so we >> can use it inside the drivers. >> >> Fixes 298 dEQP-GLES31.functional.atomic_counter.* tests and >> 9 dEQP-GLES31.functional.synchronization.inter_call. >> without_memory_barrier.ssbo_atomic_counter_mixed_dispatch_* >> tests for i965 driver. > > Hi Samuel, > > I've sent a number of patches trying to fix the problem with the > binding without reintroducing the buffer_index field. >
Great! > You can see the latest attempt here [1] based on feedback from curro > and Ilia, also see the bug for a history of the problem. > I will take a look at it later. Thanks! Sam > Tim > > [1] http://patchwork.freedesktop.org/patch/60989/ > > > >> >> Signed-off-by: Samuel Iglesias Gonsalvez <sigles...@igalia.com> >> --- >> src/glsl/ir.h | 1 + >> src/glsl/link_atomics.cpp | 32 ++++++++++++++++++++++++++++++++ >> src/glsl/linker.cpp | 1 + >> src/glsl/linker.h | 3 +++ >> src/glsl/nir/glsl_to_nir.cpp | 3 +-- >> 5 files changed, 38 insertions(+), 2 deletions(-) >> >> diff --git a/src/glsl/ir.h b/src/glsl/ir.h >> index 9c9f22d..d120203 100644 >> --- a/src/glsl/ir.h >> +++ b/src/glsl/ir.h >> @@ -851,6 +851,7 @@ public: >> * Location an atomic counter is stored at. >> */ >> struct { >> + unsigned buffer_index; >> unsigned offset; >> } atomic; >> >> diff --git a/src/glsl/link_atomics.cpp b/src/glsl/link_atomics.cpp >> index 70ef0e1..d19cd81 100644 >> --- a/src/glsl/link_atomics.cpp >> +++ b/src/glsl/link_atomics.cpp >> @@ -193,6 +193,38 @@ namespace { >> } >> } >> >> +/** >> + * Scan the program for atomic counters and store buffer index >> + * information into the variable data structure. >> + */ >> +void >> +link_assign_atomic_counter_variable_buffer_index(struct >> gl_shader_program *prog) >> +{ >> + for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) { >> + gl_shader *sh = prog->_LinkedShaders[i]; >> + >> + if (sh == NULL) >> + continue; >> + >> + foreach_in_list(ir_instruction, node, sh->ir) { >> + ir_variable *var = node->as_variable(); >> + >> + if (var && var->data.mode == ir_var_uniform && >> + var->type->contains_atomic()) { >> + for(unsigned i = 0; i < prog->NumAtomicBuffers; i++) { >> + if (prog->AtomicBuffers[i].Binding == (unsigned) var >> ->data.binding) { >> + /* Save the buffer block index that corresponds to >> variable's >> + * binding. >> + */ >> + var->data.atomic.buffer_index = i; >> + break; >> + } >> + } >> + } >> + } >> + } >> +} >> + >> void >> link_assign_atomic_counter_resources(struct gl_context *ctx, >> struct gl_shader_program *prog) >> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp >> index 424b92a..c8334f6 100644 >> --- a/src/glsl/linker.cpp >> +++ b/src/glsl/linker.cpp >> @@ -4361,6 +4361,7 @@ link_shaders(struct gl_context *ctx, struct >> gl_shader_program *prog) >> update_array_sizes(prog); >> link_assign_uniform_locations(prog, ctx >> ->Const.UniformBooleanTrue); >> link_assign_atomic_counter_resources(ctx, prog); >> + link_assign_atomic_counter_variable_buffer_index(prog); >> store_fragdepth_layout(prog); >> >> link_calculate_subroutine_compat(prog); >> diff --git a/src/glsl/linker.h b/src/glsl/linker.h >> index c80be1c..f16dd64 100644 >> --- a/src/glsl/linker.h >> +++ b/src/glsl/linker.h >> @@ -79,6 +79,9 @@ validate_interstage_uniform_blocks(struct >> gl_shader_program *prog, >> gl_shader **stages, int >> num_stages); >> >> extern void >> +link_assign_atomic_counter_variable_buffer_index(struct >> gl_shader_program *prog); >> + >> +extern void >> link_assign_atomic_counter_resources(struct gl_context *ctx, >> struct gl_shader_program >> *prog); >> >> diff --git a/src/glsl/nir/glsl_to_nir.cpp >> b/src/glsl/nir/glsl_to_nir.cpp >> index 9b50a93..aaeb53b 100644 >> --- a/src/glsl/nir/glsl_to_nir.cpp >> +++ b/src/glsl/nir/glsl_to_nir.cpp >> @@ -392,8 +392,7 @@ 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.buffer_index = ir->data.atomic.buffer_index; >> 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; > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
