[Mesa-dev] [PATCH 10/10] i965/fs: Do not mark used surfaces in FS_OPCODE_FB_WRITE/FS_OPCODE_REP_FB_WRITE
Francisco Jerez
currojerez at riseup.net
Fri Oct 30 07:10:59 PDT 2015
Iago Toral Quiroga <itoral at igalia.com> writes:
> Do it in the visitor, like we do for other opcodes.
Hm... I'm not 100% convinced of this and the texturing changes (patches
3 and 5). It definitely makes sense to do this explicitly in the
visitor for the pull constant and dataport surface opcodes, because they
take a (potentially non-constant) surface index as argument. However
for framebuffer writes, textures and the shader-time opcode, the binding
table index is calculated implicitly in the generator, so it seems kind
of dubious to have to re-do the BTI calculation in two different places.
Ideally framebuffer writes and texturing opcodes would take the surface
index explicitly as a register source, and backend_instruction::target
would die (along with the other ad-hoc fields of the IR instruction
classes that are only used for a couple of opcodes and otherwise are
just there to cause confusion and waste memory). Moving the whole BTI
computation into the visitor may be beneficial on its own for scheduling
and optimization in cases where the surface index is not a constant.
Regarding SHADER_TIME_ADD, it would probably make more sense to get rid
of it altogether, it's little more than UNTYPED_ATOMIC with exec_size =
1...
> ---
> src/mesa/drivers/dri/i965/brw_fs.cpp | 5 +++++
> src/mesa/drivers/dri/i965/brw_fs.h | 3 ++-
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 2 --
> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 24 +++++++++++++++---------
> 4 files changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 1cfae5c..1461917 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -2644,6 +2644,7 @@ void
> fs_visitor::emit_repclear_shader()
> {
> brw_wm_prog_key *key = (brw_wm_prog_key*) this->key;
> + brw_wm_prog_data *prog_data = (brw_wm_prog_data *) this->prog_data;
> int base_mrf = 1;
> int color_mrf = base_mrf + 2;
>
> @@ -2659,6 +2660,8 @@ fs_visitor::emit_repclear_shader()
> write->target = 0;
> write->header_size = 0;
> write->mlen = 1;
> + brw_mark_surface_used(&prog_data->base,
> + prog_data->binding_table.render_target_start);
> } else {
> assume(key->nr_color_regions > 0);
> for (int i = 0; i < key->nr_color_regions; ++i) {
> @@ -2668,6 +2671,8 @@ fs_visitor::emit_repclear_shader()
> write->target = i;
> write->header_size = 2;
> write->mlen = 3;
> + brw_mark_surface_used(&prog_data->base,
> + prog_data->binding_table.render_target_start + i);
> }
> }
> write->eot = true;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index 8058b34..8572e39 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -278,7 +278,8 @@ public:
> void emit_alpha_test();
> fs_inst *emit_single_fb_write(const brw::fs_builder &bld,
> fs_reg color1, fs_reg color2,
> - fs_reg src0_alpha, unsigned components);
> + fs_reg src0_alpha, unsigned components,
> + unsigned target);
> void emit_fb_writes();
> void emit_urb_writes();
> void emit_cs_terminate();
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index aafac99..5f87b06 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -249,8 +249,6 @@ fs_generator::fire_fb_write(fs_inst *inst,
> inst->eot,
> last_render_target,
> inst->header_size != 0);
> -
> - brw_mark_surface_used(&prog_data->base, surf_index);
> }
>
> void
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 5c57944..a05788f 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -462,8 +462,13 @@ fs_visitor::emit_dummy_fs()
> fs_reg(color[i]));
> }
>
> + brw_wm_prog_data *wm_prog_data = (brw_wm_prog_data *) this->prog_data;
> +
> fs_inst *write;
> write = bld.emit(FS_OPCODE_FB_WRITE);
> + brw_mark_surface_used(&wm_prog_data->base,
> + wm_prog_data->binding_table.render_target_start);
> +
> write->eot = true;
> if (devinfo->gen >= 6) {
> write->base_mrf = 2;
> @@ -477,7 +482,6 @@ fs_visitor::emit_dummy_fs()
> /* Tell the SF we don't have any inputs. Gen4-5 require at least one
> * varying to avoid GPU hangs, so set that.
> */
> - brw_wm_prog_data *wm_prog_data = (brw_wm_prog_data *) this->prog_data;
> wm_prog_data->num_varying_inputs = devinfo->gen < 6 ? 1 : 0;
> memset(wm_prog_data->urb_setup, -1,
> sizeof(wm_prog_data->urb_setup[0]) * VARYING_SLOT_MAX);
> @@ -688,7 +692,8 @@ fs_visitor::emit_alpha_test()
> fs_inst *
> fs_visitor::emit_single_fb_write(const fs_builder &bld,
> fs_reg color0, fs_reg color1,
> - fs_reg src0_alpha, unsigned components)
> + fs_reg src0_alpha, unsigned components,
> + unsigned target)
> {
> assert(stage == MESA_SHADER_FRAGMENT);
> brw_wm_prog_data *prog_data = (brw_wm_prog_data*) this->prog_data;
> @@ -722,6 +727,10 @@ fs_visitor::emit_single_fb_write(const fs_builder &bld,
> write->flag_subreg = 1;
> }
>
> + write->target = target;
> + brw_mark_surface_used(&prog_data->base,
> + prog_data->binding_table.render_target_start + target);
> +
> return write;
> }
>
> @@ -758,12 +767,11 @@ fs_visitor::emit_fb_writes()
> const fs_builder abld = bld.annotate("FB dual-source write");
>
> inst = emit_single_fb_write(abld, this->outputs[0],
> - this->dual_src_output, reg_undef, 4);
> - inst->target = 0;
> + this->dual_src_output, reg_undef, 4, 0);
>
> prog_data->dual_src_blend = true;
> } else {
> - for (int target = 0; target < key->nr_color_regions; target++) {
> + for (unsigned target = 0; target < key->nr_color_regions; target++) {
> /* Skip over outputs that weren't written. */
> if (this->outputs[target].file == BAD_FILE)
> continue;
> @@ -777,8 +785,7 @@ fs_visitor::emit_fb_writes()
>
> inst = emit_single_fb_write(abld, this->outputs[target], reg_undef,
> src0_alpha,
> - this->output_components[target]);
> - inst->target = target;
> + this->output_components[target], target);
> }
> }
>
> @@ -795,8 +802,7 @@ fs_visitor::emit_fb_writes()
> const fs_reg tmp = bld.vgrf(BRW_REGISTER_TYPE_UD, 4);
> bld.LOAD_PAYLOAD(tmp, srcs, 4, 0);
>
> - inst = emit_single_fb_write(bld, tmp, reg_undef, reg_undef, 4);
> - inst->target = 0;
> + inst = emit_single_fb_write(bld, tmp, reg_undef, reg_undef, 4, 0);
> }
>
> inst->eot = true;
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151030/90ccd99a/attachment.sig>
More information about the mesa-dev
mailing list