<div dir="ltr">On 2 December 2013 11:39, Francisco Jerez <<a href="mailto:currojerez@riseup.net" target="_blank">currojerez@riseup.net</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Until now atomic counter built-ins were handled in a way that prevented the visitor from encountering atomic counter IR variables and dereferences directly. In the new surface lowering code it's going to be more convenient to be able to call back into the visitor to let it handle the ugly details of atomic counter array dereferences, and it will make sharing the rest of the atomic intrinsic handling code easier. </blockquote><div> </div><div>The commit message should note that this patch fixes the fs part of the regression to atomic counters introduced by patch 10, by re-introducing the call to brw_mark_surface_used(). </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> --- src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 102 ++++++++++++++++----------- 1 file changed, 60 insertions(+), 42 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp index b5957c6..d65809f 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp @@ -102,34 +102,40 @@ fs_visitor::visit(ir_variable *ir) } } } else if (ir->mode == ir_var_uniform) { - int param_index = uniforms; - - /* Thanks to the lower_ubo_reference pass, we will see only - * ir_binop_ubo_load expressions and not ir_dereference_variable for UBO - * variables, so no need for them to be in variable_ht. - * - * Atomic counters take no uniform storage, no need to do - * anything here. - */ - if (ir->is_in_uniform_block() || ir->type->contains_atomic()) + if (ir->is_in_uniform_block()) { + /* Thanks to the lower_ubo_reference pass, we will see only + * ir_binop_ubo_load expressions and not ir_dereference_variable for UBO + * variables, so no need for them to be in variable_ht. + */ return; - if (dispatch_width == 16) { - if (!variable_storage(ir)) { - fail("Failed to find uniform '%s' in 16-wide\n", ir->name); - } - return; - } + } else if (ir->type->contains_atomic()) { + reg = new(this->mem_ctx) fs_reg(ir->atomic.offset); + + brw_mark_surface_used(stage_prog_data, + stage_prog_data->binding_table.abo_start + + ir->atomic.buffer_index); - param_size[param_index] = type_size(ir->type); - if (!strncmp(ir->name, "gl_", 3)) { - setup_builtin_uniform_values(ir); } else { - setup_uniform_values(ir); - } + int param_index = uniforms; - reg = new(this->mem_ctx) fs_reg(UNIFORM, param_index); - reg->type = brw_type_for_base_type(ir->type); + if (dispatch_width == 16) { + if (!variable_storage(ir)) { + fail("Failed to find uniform '%s' in 16-wide\n", ir->name); + } + return; + } + + param_size[param_index] = type_size(ir->type); + if (!strncmp(ir->name, "gl_", 3)) { + setup_builtin_uniform_values(ir); + } else { + setup_uniform_values(ir); + } + + reg = new(this->mem_ctx) fs_reg(UNIFORM, param_index); + reg->type = brw_type_for_base_type(ir->type); + } } else if (ir->mode == ir_var_system_value) { if (ir->location == SYSTEM_VALUE_SAMPLE_POS) { @@ -182,31 +188,43 @@ fs_visitor::visit(ir_dereference_array *ir) src = this->result; src.type = brw_type_for_base_type(ir->type); - if (constant_index) { - assert(src.file == UNIFORM || src.file == GRF); - src.reg_offset += constant_index->value.i[0] * element_size; - } else { - /* Variable index array dereference. We attach the variable index - * component to the reg as a pointer to a register containing the - * offset. Currently only uniform arrays are supported in this patch, - * and that reladdr pointer is resolved by - * move_uniform_array_access_to_pull_constants(). All other array types - * are lowered by lower_variable_index_to_cond_assign(). - */ + if (ir->array->type->contains_atomic()) { + fs_reg tmp(this, glsl_type::uint_type); + ir->array_index->accept(this); - fs_reg index_reg; - index_reg = fs_reg(this, glsl_type::int_type); - emit(BRW_OPCODE_MUL, index_reg, this->result, fs_reg(element_size)); + emit(MUL(tmp, this->result, ATOMIC_COUNTER_SIZE)); + emit(ADD(tmp, tmp, src)); + this->result = tmp; + </blockquote><div> </div><div>I believe we need a call to brw_mark_surface_used() here too. </div><div>With those issues addressed, this patch is: Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div></div></div></div>