[Mesa-dev] [PATCH 4/6] i965/fs: Simplify interference scan in register coalescing.

[Kenneth Graunke]

On 04/16/2014 11:07 AM, Matt Turner wrote:
> We were starting at the beginning of the instruction list, rather than
> with the MOV instruction itself. This allows us to coalesce after
> control flow.
> 
> Excluding the shaders from an unreleased title, the shader-db results:
> 
> total instructions in shared programs: 1603791 -> 1594215 (-0.60%)
> instructions in affected programs:     678772 -> 669196 (-1.41%)
> GAINED:                                5
> LOST:                                  0
> ---
>  .../drivers/dri/i965/brw_fs_register_coalesce.cpp  | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp b/src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp
> index 5836fb7..58f6ca7 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp
> @@ -73,7 +73,7 @@ is_coalesce_candidate(const fs_inst *inst, const int *virtual_grf_sizes)
>  
>  static bool
>  can_coalesce_vars(brw::fs_live_variables *live_intervals,
> -                  const exec_list *instructions, const fs_inst *inst,
> +                  const exec_list *instructions, const fs_inst *inst, int ip,
>                    int var_to, int var_from)
>  {
>     if (!live_intervals->vars_interfere(var_from, var_to))
> @@ -92,21 +92,15 @@ can_coalesce_vars(brw::fs_live_variables *live_intervals,
>     if (live_intervals->end[var_to] > live_intervals->end[var_from])
>        return false;
>  
> -   int scan_ip = -1;
> -
> -   foreach_list(n, instructions) {
> -      fs_inst *scan_inst = (fs_inst *)n;
> -      scan_ip++;
> +   assert(ip >= live_intervals->start[var_to]);
>  
> +   fs_inst *scan_inst;
> +   for (scan_inst = (fs_inst *)inst->next;
> +        scan_inst->next != NULL && ip <= live_intervals->end[var_to];

This doesn't seem quite right to me.  The last instruction in the list
will have scan_inst->next == the tail sentinel (so, ->next->next ==
NULL).  I think you want:

   !scan_inst->is_tail_sentinel() && ip <= live_intervals->end[var_to];

With that changed (or my logic rebuffed), this is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

> +        scan_inst = (fs_inst *)scan_inst->next, ip++) {
>        if (scan_inst->is_control_flow())
>           return false;
>  
> -      if (scan_ip <= live_intervals->start[var_to])
> -         continue;
> -
> -      if (scan_ip > live_intervals->end[var_to])
> -         break;
> -
>        if (scan_inst->dst.equals(inst->dst) ||
>            scan_inst->dst.equals(inst->src[0]))
>           return false;
> @@ -129,9 +123,11 @@ fs_visitor::register_coalesce()
>     fs_inst *mov[MAX_SAMPLER_MESSAGE_SIZE];
>     int var_to[MAX_SAMPLER_MESSAGE_SIZE];
>     int var_from[MAX_SAMPLER_MESSAGE_SIZE];
> +   int ip = -1;
>  
>     foreach_list(node, &this->instructions) {
>        fs_inst *inst = (fs_inst *)node;
> +      ip++;
>  
>        if (!is_coalesce_candidate(inst, virtual_grf_sizes))
>           continue;
> @@ -170,7 +166,7 @@ fs_visitor::register_coalesce()
>           var_to[i] = live_intervals->var_from_vgrf[reg_to] + reg_to_offset[i];
>           var_from[i] = live_intervals->var_from_vgrf[reg_from] + i;
>  
> -         if (!can_coalesce_vars(live_intervals, &instructions, inst,
> +         if (!can_coalesce_vars(live_intervals, &instructions, inst, ip,
>                                  var_to[i], var_from[i])) {
>              can_coalesce = false;
>              reg_from = -1;
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140417/86b3b74f/attachment-0001.sig>