[Mesa-dev] [PATCH 01/21] i965/fs: Get rid of fs_visitor::do_dual_src.

[Anuj Phogat]

On Fri, Jul 22, 2016 at 8:58 PM, Francisco Jerez <currojerez at riseup.net> wrote:
> This boolean flag was being used for two different things:
>
>  - To set the brw_wm_prog_data::dual_src_blend flag.  Instead we can
>    just set it based on whether the dual_src_output register is valid,
>    which will be the case if the shader writes the secondary blending
>    color.
>
>  - To decide whether to call emit_single_fb_write() once, or in a loop
>    that would iterate only once, which seems pretty useless.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.h           |  1 -
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp     |  2 --
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 37 +++++++++++-----------------
>  3 files changed, 14 insertions(+), 26 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index fc1e1c4..46b15b4 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -318,7 +318,6 @@ public:
>     fs_reg sample_mask;
>     fs_reg outputs[VARYING_SLOT_MAX];
>     fs_reg dual_src_output;
> -   bool do_dual_src;
>     int first_non_payload_grf;
>     /** Either BRW_MAX_GRF or GEN7_MRF_HACK_START */
>     unsigned max_grf;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index 50d73eb..2872b2d 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -103,12 +103,10 @@ fs_visitor::nir_setup_outputs()
>           if (key->force_dual_color_blend &&
>               var->data.location == FRAG_RESULT_DATA1) {
>              this->dual_src_output = reg;
> -            this->do_dual_src = true;
>           } else if (var->data.index > 0) {
>              assert(var->data.location == FRAG_RESULT_DATA0);
>              assert(var->data.index == 1);
>              this->dual_src_output = reg;
> -            this->do_dual_src = true;
>           } else if (var->data.location == FRAG_RESULT_COLOR) {
>              /* Writing gl_FragColor outputs to all color regions. */
>              for (unsigned int i = 0; i < MAX2(key->nr_color_regions, 1); i++) {
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 6d84374..808d8af 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -437,33 +437,25 @@ fs_visitor::emit_fb_writes()
>                             "in SIMD16+ mode.\n");
>     }
>
> -   if (do_dual_src) {
> -      const fs_builder abld = bld.annotate("FB dual-source write");
> +   for (int target = 0; target < key->nr_color_regions; target++) {
> +      /* Skip over outputs that weren't written. */
> +      if (this->outputs[target].file == BAD_FILE)
> +         continue;
>
> -      inst = emit_single_fb_write(abld, this->outputs[0],
> -                                  this->dual_src_output, reg_undef, 4);
> -      inst->target = 0;
> -
> -      prog_data->dual_src_blend = true;
> -   } else {
> -      for (int target = 0; target < key->nr_color_regions; target++) {
> -         /* Skip over outputs that weren't written. */
> -         if (this->outputs[target].file == BAD_FILE)
> -            continue;
> +      const fs_builder abld = bld.annotate(
> +         ralloc_asprintf(this->mem_ctx, "FB write target %d", target));
>
> -         const fs_builder abld = bld.annotate(
> -            ralloc_asprintf(this->mem_ctx, "FB write target %d", target));
> +      fs_reg src0_alpha;
> +      if (devinfo->gen >= 6 && key->replicate_alpha && target != 0)
> +         src0_alpha = offset(outputs[0], bld, 3);
>
> -         fs_reg src0_alpha;
> -         if (devinfo->gen >= 6 && key->replicate_alpha && target != 0)
> -            src0_alpha = offset(outputs[0], bld, 3);
> -
> -         inst = emit_single_fb_write(abld, this->outputs[target], reg_undef,
> -                                     src0_alpha, 4);
> -         inst->target = target;
> -      }
> +      inst = emit_single_fb_write(abld, this->outputs[target],
> +                                  this->dual_src_output, src0_alpha, 4);
> +      inst->target = target;
>     }
>
> +   prog_data->dual_src_blend = (this->dual_src_output.file != BAD_FILE);
> +
It'll be nice to add this assert here:
assert(!prog_data->dual_src_blend ||  key->nr_color_regions == 1);

>     if (inst == NULL) {
>        /* Even if there's no color buffers enabled, we still need to send
>         * alpha out the pipeline to our null renderbuffer to support
> @@ -914,7 +906,6 @@ fs_visitor::init()
>     this->promoted_constants = 0,
>
>     this->spilled_any_registers = false;
> -   this->do_dual_src = false;
>  }
>
>  fs_visitor::~fs_visitor()
> --
> 2.9.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Patch is:
Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>