[Mesa-dev] [PATCH 5/5] i965/fs: Use const fs_reg & rather than a copy or pointer.

[Kenneth Graunke]

On Wednesday, October 29, 2014 02:10:13 PM Matt Turner wrote:
> Also while we're touching var_from_reg, just make it an inline function.
> ---
>  src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp  |  8 ++++----
>  src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp       | 14 
++++----------
>  src/mesa/drivers/dri/i965/brw_fs_live_variables.h         | 11 ++++++++---
>  src/mesa/drivers/dri/i965/brw_fs_saturate_propagation.cpp |  2 +-
>  4 files changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
> index 414c4a0..2b26177 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
> @@ -56,10 +56,10 @@ fs_visitor::dead_code_eliminate()
>              bool result_live = false;
>  
>              if (inst->regs_written == 1) {
> -               int var = live_intervals->var_from_reg(&inst->dst);
> +               int var = live_intervals->var_from_reg(inst->dst);
>                 result_live = BITSET_TEST(live, var);
>              } else {
> -               int var = live_intervals->var_from_reg(&inst->dst);
> +               int var = live_intervals->var_from_reg(inst->dst);
>                 for (int i = 0; i < inst->regs_written; i++) {
>                    result_live = result_live || BITSET_TEST(live, var + i);
>                 }
> @@ -86,7 +86,7 @@ fs_visitor::dead_code_eliminate()
>  
>           if (inst->dst.file == GRF) {
>              if (!inst->is_partial_write()) {
> -               int var = live_intervals->var_from_reg(&inst->dst);
> +               int var = live_intervals->var_from_reg(inst->dst);
>                 for (int i = 0; i < inst->regs_written; i++) {
>                    BITSET_CLEAR(live, var + i);
>                 }
> @@ -99,7 +99,7 @@ fs_visitor::dead_code_eliminate()
>  
>           for (int i = 0; i < inst->sources; i++) {
>              if (inst->src[i].file == GRF) {
> -               int var = live_intervals->var_from_reg(&inst->src[i]);
> +               int var = live_intervals->var_from_reg(inst->src[i]);
>  
>                 for (int j = 0; j < inst->regs_read(this, i); j++) {
>                    BITSET_SET(live, var + j);
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp
> index dbe1d34..b5c81cc 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp
> @@ -54,9 +54,9 @@ using namespace brw;
>  
>  void
>  fs_live_variables::setup_one_read(struct block_data *bd, fs_inst *inst,
> -                                  int ip, fs_reg reg)
> +                                  int ip, const fs_reg &reg)
>  {
> -   int var = var_from_reg(&reg);
> +   int var = var_from_reg(reg);
>     assert(var < num_vars);
>  
>     /* In most cases, a register can be written over safely by the
> @@ -106,9 +106,9 @@ fs_live_variables::setup_one_read(struct block_data *bd, 
fs_inst *inst,
>  
>  void
>  fs_live_variables::setup_one_write(struct block_data *bd, fs_inst *inst,
> -                                   int ip, fs_reg reg)
> +                                   int ip, const fs_reg &reg)
>  {
> -   int var = var_from_reg(&reg);
> +   int var = var_from_reg(reg);
>     assert(var < num_vars);
>  
>     start[var] = MIN2(start[var], ip);
> @@ -272,12 +272,6 @@ fs_live_variables::compute_start_end()
>     }
>  }
>  
> -int
> -fs_live_variables::var_from_reg(fs_reg *reg)
> -{
> -   return var_from_vgrf[reg->reg] + reg->reg_offset;
> -}
> -
>  fs_live_variables::fs_live_variables(fs_visitor *v, const cfg_t *cfg)
>     : v(v), cfg(cfg)
>  {
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_live_variables.h 
b/src/mesa/drivers/dri/i965/brw_fs_live_variables.h
> index 2bfb583..a52f922 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_live_variables.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs_live_variables.h
> @@ -66,7 +66,10 @@ public:
>     ~fs_live_variables();
>  
>     bool vars_interfere(int a, int b);
> -   int var_from_reg(fs_reg *reg);
> +   int var_from_reg(const fs_reg &reg) const
> +   {
> +      return var_from_vgrf[reg.reg] + reg.reg_offset;
> +   }
>  
>     /** Map from virtual GRF number to index in block_data arrays. */
>     int *var_from_vgrf;
> @@ -96,8 +99,10 @@ public:
>  
>  protected:
>     void setup_def_use();
> -   void setup_one_read(struct block_data *bd, fs_inst *inst, int ip, fs_reg 
reg);
> -   void setup_one_write(struct block_data *bd, fs_inst *inst, int ip, 
fs_reg reg);
> +   void setup_one_read(struct block_data *bd, fs_inst *inst, int ip,
> +                       const fs_reg &reg);
> +   void setup_one_write(struct block_data *bd, fs_inst *inst, int ip,
> +                        const fs_reg &reg);
>     void compute_live_variables();
>     void compute_start_end();
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_saturate_propagation.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_saturate_propagation.cpp
> index 347a78e..a4145ac 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_saturate_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_saturate_propagation.cpp
> @@ -45,7 +45,7 @@ opt_saturate_propagation_local(fs_visitor *v, bblock_t 
*block)
>            !inst->saturate)
>           continue;
>  
> -      int src_var = v->live_intervals->var_from_reg(&inst->src[0]);
> +      int src_var = v->live_intervals->var_from_reg(inst->src[0]);
>        int src_end_ip = v->live_intervals->end[src_var];
>  
>        bool interfered = false;
> 

Yup, I'm definitely a fan of const references rather than passing by pointer.

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141108/8f9fefdd/attachment.sig>