On Tue, Sep 29, 2015 at 2:05 AM, Timothy Arceri <t_arc...@yahoo.com.au> wrote: > On Tue, 2015-09-29 at 01:05 -0400, Ilia Mirkin wrote: >> On Mon, Sep 28, 2015 at 10:42 PM, Timothy Arceri < >> t_arc...@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. >> > >> > 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 <apinhe...@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 f03a107..30726be 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 4f45770..670e39e 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; >> > >> > @@ -1880,7 +1879,8 @@ void nir_lower_clip_fs(nir_shader *shader, >> > unsigned ucp_enables); >> > >> > void nir_lower_two_sided_color(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); >> > >> > bool nir_lower_gs_intrinsics(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 1d4f6ab..22909a1 100644 >> > --- a/src/mesa/drivers/dri/i965/brw_nir.c >> > +++ b/src/mesa/drivers/dri/i965/brw_nir.c >> > @@ -155,8 +155,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 63c40ba..a74eb86 100644 >> > --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp >> > +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp >> > @@ -2435,7 +2435,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); >> >> Shouldn't there be a similar adjustment in brw_upload_abo_surfaces? >> (Although I admit to not fully understanding this stuff, so very much >> an open question.) > > Which part are you talking about? > > I assume you mean this line: > > struct gl_atomic_buffer_binding *binding = > &ctx->AtomicBufferBindings[prog->AtomicBuffers[i].Binding] > > It looks fine to me. > > prog->AtomicBuffers[i].Binding gets the binding for each active buffer > and then uses the binding as the index to AtomicBufferBindings to look > up the buffer object which was set in bind_atomic_buffer() using the > binding as the index.
No. I mean the line: brw->vtbl.emit_buffer_surface_state(brw, &surf_offsets[i], bo, Shouldn't that look up the atomic buffer index in prog->Uniforms instead of using 'i'? > > >> >> > >> > /* Calculate the surface offset */ >> > src_reg offset(this, glsl_type::uint_type); >> > -- >> > 2.4.3 >> > >> > _______________________________________________ >> > mesa-dev mailing list >> > mesa-dev@lists.freedesktop.org >> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev