[Mesa-dev] [PATCH] i965/vec4: check writemask when bailing out at register coalesce

[Matt Turner]

On Fri, Sep 11, 2015 at 5:18 AM, Alejandro Piñeiro <apinheiro at igalia.com> wrote:
> opt_register_coalesce stopped to check previous instructions to
> coalesce with if somebody else was writing on the same
> destination. This can be optimized to check if somebody else was
> writing to the same channels of the same destination using the
> writemask.
>
> Shader DB results (taking into account only vec4):
>
> total instructions in shared programs: 1781593 -> 1734957 (-2.62%)
> instructions in affected programs:     1238390 -> 1191754 (-3.77%)
> helped:                                12782

Heh, yeah, that seems like a pretty obvious improvement. Nice :)

> HURT:                                  0
> GAINED:                                0
> LOST:                                  0
> ---
>
> piglit run on both IR and NIR path without any regression.
>
>
>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 85dc372..998dbf2 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -1089,11 +1089,12 @@ vec4_visitor::opt_register_coalesce()
>          if (interfered)
>             break;
>
> -         /* If somebody else writes our destination here, we can't coalesce
> -          * before that.
> +         /* If somebody else writes the same channels of our destination here,
> +          * we can't coalesce before that.
>            */
> -         if (inst->dst.in_range(scan_inst->dst, scan_inst->regs_written))
> -           break;
> +         if ((inst->dst.in_range(scan_inst->dst, scan_inst->regs_written) &&
> +              (inst->dst.writemask & scan_inst->dst.writemask)))

Too many parentheses, and the indentation is a little off I think. How about:

  if (inst->dst.in_range(scan_inst->dst, scan_inst->regs_written) &&
      (inst->dst.writemask & scan_inst->dst.writemask) != 0)


With that,

Reviewed-by: Matt Turner <mattst88 at gmail.com>