[Mesa-dev] [PATCH 14/29] i965: Lift the constness restriction on surface indices passed to untyped ops.

[Kenneth Graunke]

On Saturday, May 02, 2015 06:29:41 PM Francisco Jerez wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp   | 8 ++------
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp     | 4 ++++
>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 8 ++------
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp   | 4 ++++
>  4 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index d476c92..6dd14c2 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -2015,19 +2015,15 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
>           break;
>  
>        case SHADER_OPCODE_UNTYPED_ATOMIC:
> -         assert(src[1].file == BRW_IMMEDIATE_VALUE &&
> -                src[2].file == BRW_IMMEDIATE_VALUE);
> +         assert(src[2].file == BRW_IMMEDIATE_VALUE);
>           brw_untyped_atomic(p, dst, src[0], src[1], src[2].dw1.ud,
>                              inst->mlen, !inst->dst.is_null());
> -         brw_mark_surface_used(prog_data, src[1].dw1.ud);
>           break;
>  
>        case SHADER_OPCODE_UNTYPED_SURFACE_READ:
> -         assert(src[1].file == BRW_IMMEDIATE_VALUE &&
> -                src[2].file == BRW_IMMEDIATE_VALUE);
> +         assert(src[2].file == BRW_IMMEDIATE_VALUE);
>           brw_untyped_surface_read(p, dst, src[0], src[1],
>                                    inst->mlen, src[2].dw1.ud);
> -         brw_mark_surface_used(prog_data, src[1].dw1.ud);
>           break;
>  
>        case SHADER_OPCODE_UNTYPED_SURFACE_WRITE:
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 12d02d3..9c394ba 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -157,6 +157,10 @@ fs_visitor::visit(ir_variable *ir)
>        if (ir->type->contains_atomic()) {
>           reg = new(this->mem_ctx) fs_reg(ir->data.atomic.offset);
>  
> +         brw_mark_surface_used(stage_prog_data,
> +                               stage_prog_data->binding_table.abo_start +
> +                               ir->data.binding);
> +
>        } else if (ir->is_in_uniform_block() || type_size(ir->type) == 0) {
>           /* Thanks to the lower_ubo_reference pass, we will see only
>            * ir_binop_ubo_load expressions and not ir_dereference_variable for UBO
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> index 9d37c93..b8f546f 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> @@ -1469,19 +1469,15 @@ vec4_generator::generate_code(const cfg_t *cfg)
>           break;
>  
>        case SHADER_OPCODE_UNTYPED_ATOMIC:
> -         assert(src[1].file == BRW_IMMEDIATE_VALUE &&
> -                src[2].file == BRW_IMMEDIATE_VALUE);
> +         assert(src[2].file == BRW_IMMEDIATE_VALUE);
>           brw_untyped_atomic(p, dst, src[0], src[1], src[2].dw1.ud, inst->mlen,
>                              !inst->dst.is_null());
> -         brw_mark_surface_used(&prog_data->base, src[1].dw1.ud);
>           break;
>  
>        case SHADER_OPCODE_UNTYPED_SURFACE_READ:
> -         assert(src[1].file == BRW_IMMEDIATE_VALUE &&
> -                src[2].file == BRW_IMMEDIATE_VALUE);
> +         assert(src[2].file == BRW_IMMEDIATE_VALUE);
>           brw_untyped_surface_read(p, dst, src[0], src[1], inst->mlen,
>                                    src[2].dw1.ud);
> -         brw_mark_surface_used(&prog_data->base, src[1].dw1.ud);
>           break;
>  
>        case SHADER_OPCODE_UNTYPED_SURFACE_WRITE:
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 22586de..1f87d99 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -1069,6 +1069,10 @@ vec4_visitor::visit(ir_variable *ir)
>        if (ir->type->contains_atomic()) {
>           reg = new(this->mem_ctx) dst_reg(ir->data.atomic.offset);
>  
> +         brw_mark_surface_used(stage_prog_data,
> +                               stage_prog_data->binding_table.abo_start +
> +                               ir->data.binding);
> +
>        } else if (ir->is_in_uniform_block() || type_size(ir->type) == 0) {
>           /* Thanks to the lower_ubo_reference pass, we will see only
>            * ir_binop_ubo_load expressions and not ir_dereference_variable for
> 

This change looks good, but don't you need to make brw_fs_nir.cpp call
brw_mark_surface_used() as well?  It supports atomic counters, today.

With that fixed (or an explanation why it's unnecessary), this is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150504/3b50f6f3/attachment.sig>