[Mesa-dev] [PATCH 4/5] i965/fs.h: Refactor tests for instructions modifying a register.

[Kenneth Graunke]

On 07/09/2012 03:40 PM, Eric Anholt wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp               |   25 ++++++++++----------
>  src/mesa/drivers/dri/i965/brw_fs.h                 |    1 +
>  .../drivers/dri/i965/brw_fs_copy_propagation.cpp   |   16 ++-----------
>  src/mesa/drivers/dri/i965/brw_fs_cse.cpp           |    8 +------
>  4 files changed, 16 insertions(+), 34 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 374a059..354134c 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -168,6 +168,15 @@ fs_inst::regs_written()
>  }
>  
>  bool
> +fs_inst::overwrites_reg(const fs_reg &reg)
> +{
> +   return (reg.file == dst.file &&
> +           reg.reg == dst.reg &&
> +           reg.reg_offset >= dst.reg_offset  &&
> +           reg.reg_offset < dst.reg_offset + regs_written());
> +}
> +
> +bool
>  fs_inst::is_tex()
>  {
>     return (opcode == SHADER_OPCODE_TEX ||
> @@ -1400,9 +1409,7 @@ fs_visitor::propagate_constants()
>  	 }
>  
>  	 if (scan_inst->dst.file == GRF &&
> -	     scan_inst->dst.reg == inst->dst.reg &&
> -	     (scan_inst->dst.reg_offset == inst->dst.reg_offset ||
> -	      scan_inst->regs_written() > 1)) {
> +             scan_inst->overwrites_reg(inst->dst)) {

Technically this is a change in behavior: instead of breaking whenever
regs_written() > 1, we actually check if it's in the range.

That's a good change to make, but it might be nice to note it in the
commit message.

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

>  	    break;
>  	 }
>        }
> @@ -1602,16 +1609,8 @@ fs_visitor::register_coalesce()
>  	   !scan_inst->is_tail_sentinel();
>  	   scan_inst = (fs_inst *)scan_inst->next) {
>  	 if (scan_inst->dst.file == GRF) {
> -	    if (scan_inst->dst.reg == inst->dst.reg &&
> -		(scan_inst->dst.reg_offset == inst->dst.reg_offset ||
> -		 scan_inst->regs_written() > 1)) {
> -	       interfered = true;
> -	       break;
> -	    }
> -	    if (inst->src[0].file == GRF &&
> -		scan_inst->dst.reg == inst->src[0].reg &&
> -		(scan_inst->dst.reg_offset == inst->src[0].reg_offset ||
> -		 scan_inst->regs_written() > 1)) {
> +	    if (scan_inst->overwrites_reg(inst->dst) ||
> +                scan_inst->overwrites_reg(inst->src[0])) {
>  	       interfered = true;
>  	       break;
>  	    }
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index deb58d8..6e3c46a 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -150,6 +150,7 @@ public:
>  
>     bool equals(fs_inst *inst);
>     int regs_written();
> +   bool overwrites_reg(const fs_reg &reg);
>     bool is_tex();
>     bool is_math();
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> index 106964d..1870f43 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> @@ -98,23 +98,11 @@ fs_visitor::opt_copy_propagate_local(void *mem_ctx,
>  
>        /* kill the destination from the ACP */
>        if (inst->dst.file == GRF) {
> -	 int start_offset = inst->dst.reg_offset;
> -	 int end_offset = start_offset + inst->regs_written();
> -
>  	 foreach_list_safe(entry_node, acp) {
>  	    acp_entry *entry = (acp_entry *)entry_node;
>  
> -	    if (entry->dst.file == GRF &&
> -		entry->dst.reg == inst->dst.reg &&
> -		entry->dst.reg_offset >= start_offset &&
> -		entry->dst.reg_offset < end_offset) {
> -	       entry->remove();
> -	       continue;
> -	    }
> -	    if (entry->src.file == GRF &&
> -		entry->src.reg == inst->dst.reg &&
> -		entry->src.reg_offset >= start_offset &&
> -		entry->src.reg_offset < end_offset) {
> +	    if (inst->overwrites_reg(entry->dst) ||
> +                inst->overwrites_reg(entry->src)) {
>  	       entry->remove();
>  	    }
>  	 }
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
> index fd28e14..7bf6698 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
> @@ -150,17 +150,11 @@ fs_visitor::opt_cse_local(fs_bblock *block, exec_list *aeb)
>        }
>  
>        /* Kill all AEB entries that use the destination. */
> -      int start_offset = inst->dst.reg_offset;
> -      int end_offset = start_offset + inst->regs_written();
> -
>        foreach_list_safe(entry_node, aeb) {
>  	 aeb_entry *entry = (aeb_entry *)entry_node;
>  
>  	 for (int i = 0; i < 3; i++) {
> -	    if (entry->generator->src[i].file == inst->dst.file &&
> -		entry->generator->src[i].reg == inst->dst.reg &&
> -		entry->generator->src[i].reg_offset >= start_offset &&
> -		entry->generator->src[i].reg_offset < end_offset) {
> +            if (inst->overwrites_reg(entry->generator->src[i])) {
>  	       entry->remove();
>  	       ralloc_free(entry);
>  	       break;
>