On 2 December 2013 11:39, Francisco Jerez <curroje...@riseup.net> wrote:
> ---
> src/mesa/drivers/dri/i965/brw_fs.h | 9 ------
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 38
> ++++------------------
> src/mesa/drivers/dri/i965/brw_vec4.h | 9 ------
> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 40
> ++++--------------------
> 4 files changed, 12 insertions(+), 84 deletions(-)
>
The commit message should note that you've also loosened the restrictions
on atomic operations--the surf_index used with an atomic operatoin no
longer needs to be an immediate constant.
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
> b/src/mesa/drivers/dri/i965/brw_fs.h
> index 4ada075..7bfa9fd 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -601,15 +601,6 @@ private:
> struct brw_reg offset,
> struct brw_reg value);
>
> - void generate_untyped_atomic(fs_inst *inst,
> - struct brw_reg dst,
> - struct brw_reg atomic_op,
> - struct brw_reg surf_index);
> -
> - void generate_untyped_surface_read(fs_inst *inst,
> - struct brw_reg dst,
> - struct brw_reg surf_index);
> -
> void patch_discard_jumps_to_fb_writes();
>
> struct brw_context *brw;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index 4eb651f..0d50051 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -1255,36 +1255,6 @@ fs_generator::generate_shader_time_add(fs_inst
> *inst,
> }
>
> void
> -fs_generator::generate_untyped_atomic(fs_inst *inst, struct brw_reg dst,
> - struct brw_reg atomic_op,
> - struct brw_reg surf_index)
> -{
> - assert(atomic_op.file == BRW_IMMEDIATE_VALUE &&
> - atomic_op.type == BRW_REGISTER_TYPE_UD &&
> - surf_index.file == BRW_IMMEDIATE_VALUE &&
> - surf_index.type == BRW_REGISTER_TYPE_UD);
> -
> - brw_untyped_atomic(p, dst, brw_message_reg(inst->base_mrf),
> - surf_index, atomic_op.dw1.ud,
> - inst->mlen, true);
> -
> - brw_mark_surface_used(&c->prog_data.base, surf_index.dw1.ud);
> -}
> -
> -void
> -fs_generator::generate_untyped_surface_read(fs_inst *inst, struct brw_reg
> dst,
> - struct brw_reg surf_index)
> -{
> - assert(surf_index.file == BRW_IMMEDIATE_VALUE &&
> - surf_index.type == BRW_REGISTER_TYPE_UD);
> -
> - brw_untyped_surface_read(p, dst, brw_message_reg(inst->base_mrf),
> - surf_index, inst->mlen, 1);
> -
> - brw_mark_surface_used(&c->prog_data.base, surf_index.dw1.ud);
> -}
> -
> -void
> fs_generator::generate_code(exec_list *instructions)
> {
> int last_native_insn_offset = p->next_insn_offset;
> @@ -1709,11 +1679,15 @@ fs_generator::generate_code(exec_list
> *instructions)
> break;
>
> case SHADER_OPCODE_UNTYPED_ATOMIC:
> - generate_untyped_atomic(inst, dst, src[0], src[1]);
> + assert(src[1].file == BRW_IMMEDIATE_VALUE);
> + brw_untyped_atomic(p, dst, brw_message_reg(inst->base_mrf),
> + src[0], src[1].dw1.ud, inst->mlen, true);
>
I've seen the pattern crop up several times in this series now of: 1.
assert that a brw_reg is an immediate value, 2. extract reg.dw1.ud. How
about if we make a function to do this (maybe brw_get_imm_ud())? I think
that will be a lot more readable since it won't force people to remember
that the immediate value is stored in brw_reg::dw1.
> break;
>
> case SHADER_OPCODE_UNTYPED_SURFACE_READ:
> - generate_untyped_surface_read(inst, dst, src[0]);
> + assert(src[1].file == BRW_IMMEDIATE_VALUE);
> + brw_untyped_surface_read(p, dst, brw_message_reg(inst->base_mrf),
> + src[0], inst->mlen, src[1].dw1.ud);
> break;
>
In both of the above two cases, generate_untyped_{atomic,surface_read} used
to call brw_mark_surface_used(). That's not being called anymore.
>
> case FS_OPCODE_SET_SIMD4X2_OFFSET:
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h
> b/src/mesa/drivers/dri/i965/brw_vec4.h
> index 355d497..7e07929 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
> @@ -666,15 +666,6 @@ private:
> void generate_unpack_flags(vec4_instruction *inst,
> struct brw_reg dst);
>
> - void generate_untyped_atomic(vec4_instruction *inst,
> - struct brw_reg dst,
> - struct brw_reg atomic_op,
> - struct brw_reg surf_index);
> -
> - void generate_untyped_surface_read(vec4_instruction *inst,
> - struct brw_reg dst,
> - struct brw_reg surf_index);
> -
> struct brw_context *brw;
>
> struct brw_compile *p;
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> index 3ac45a9..d29c3dd 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> @@ -847,38 +847,6 @@
> vec4_generator::generate_pull_constant_load_gen7(vec4_instruction *inst,
> brw_mark_surface_used(&prog_data->base, surf_index.dw1.ud);
> }
>
> -void
> -vec4_generator::generate_untyped_atomic(vec4_instruction *inst,
> - struct brw_reg dst,
> - struct brw_reg atomic_op,
> - struct brw_reg surf_index)
> -{
> - assert(atomic_op.file == BRW_IMMEDIATE_VALUE &&
> - atomic_op.type == BRW_REGISTER_TYPE_UD &&
> - surf_index.file == BRW_IMMEDIATE_VALUE &&
> - surf_index.type == BRW_REGISTER_TYPE_UD);
> -
> - brw_untyped_atomic(p, dst, brw_message_reg(inst->base_mrf),
> - surf_index, atomic_op.dw1.ud,
> - inst->mlen, true);
> -
> - brw_mark_surface_used(&prog_data->base, surf_index.dw1.ud);
> -}
> -
> -void
> -vec4_generator::generate_untyped_surface_read(vec4_instruction *inst,
> - struct brw_reg dst,
> - struct brw_reg surf_index)
> -{
> - assert(surf_index.file == BRW_IMMEDIATE_VALUE &&
> - surf_index.type == BRW_REGISTER_TYPE_UD);
> -
> - brw_untyped_surface_read(p, dst, brw_message_reg(inst->base_mrf),
> - surf_index, inst->mlen, 1);
> -
> - brw_mark_surface_used(&prog_data->base, surf_index.dw1.ud);
> -}
> -
> /**
> * Generate assembly for a Vec4 IR instruction.
> *
> @@ -1193,11 +1161,15 @@
> vec4_generator::generate_vec4_instruction(vec4_instruction *instruction,
> break;
>
> case SHADER_OPCODE_UNTYPED_ATOMIC:
> - generate_untyped_atomic(inst, dst, src[0], src[1]);
> + assert(src[1].file == BRW_IMMEDIATE_VALUE);
> + brw_untyped_atomic(p, dst, brw_message_reg(inst->base_mrf),
> + src[0], src[1].dw1.ud, inst->mlen, true);
> break;
>
> case SHADER_OPCODE_UNTYPED_SURFACE_READ:
> - generate_untyped_surface_read(inst, dst, src[0]);
> + assert(src[1].file == BRW_IMMEDIATE_VALUE);
> + brw_untyped_surface_read(p, dst, brw_message_reg(inst->base_mrf),
> + src[0], inst->mlen, src[1].dw1.ud);
> break;
>
Same problem about brw_mark_surface_used() here.
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
