On 2 December 2013 11:39, Francisco Jerez <curroje...@riseup.net> wrote:
> 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.
>
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().
> ---
> 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;
> +
>
I believe we need a call to brw_mark_surface_used() here too.
With those issues addressed, this patch is:
Reviewed-by: Paul Berry <stereotype...@gmail.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
