[Mesa-dev] [PATCH 12/19] i965/fs: Pass fs_regs by constant reference where possible.

[Pohjolainen, Topi]

On Thu, Feb 20, 2014 at 01:41:25PM -0800, Matt Turner wrote:
> These functions (modulo emit_lrp, necessitating the small fix-up) pass
> these arguments by value unmodified to other functions. No point in
> making an additional copy.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp         |  7 ++++---
>  src/mesa/drivers/dri/i965/brw_fs.h           | 14 ++++++++------
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 12 +++++++-----
>  3 files changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 37b5bb0..7dc83ad 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -238,8 +238,9 @@ fs_visitor::CMP(fs_reg dst, fs_reg src0, fs_reg src1, uint32_t condition)
>  }
>  
>  exec_list
> -fs_visitor::VARYING_PULL_CONSTANT_LOAD(fs_reg dst, fs_reg surf_index,
> -                                       fs_reg varying_offset,
> +fs_visitor::VARYING_PULL_CONSTANT_LOAD(const fs_reg &dst,
> +                                       const fs_reg &surf_index,
> +                                       const fs_reg &varying_offset,
>                                         uint32_t const_offset)
>  {
>     exec_list instructions;
> @@ -3191,7 +3192,7 @@ fs_visitor::dump_instruction(backend_instruction *be_inst)
>  fs_inst *
>  fs_visitor::get_instruction_generating_reg(fs_inst *start,
>  					   fs_inst *end,
> -					   fs_reg reg)
> +					   const fs_reg &reg)
>  {
>     if (end == start ||
>         end->is_partial_write() ||
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index a1f33e7..da4ef2a 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -317,10 +317,11 @@ public:
>     int type_size(const struct glsl_type *type);
>     fs_inst *get_instruction_generating_reg(fs_inst *start,
>  					   fs_inst *end,
> -					   fs_reg reg);
> +					   const fs_reg &reg);
>  
> -   exec_list VARYING_PULL_CONSTANT_LOAD(fs_reg dst, fs_reg surf_index,
> -                                        fs_reg varying_offset,
> +   exec_list VARYING_PULL_CONSTANT_LOAD(const fs_reg &dst,
> +                                        const fs_reg &surf_index,
> +                                        const fs_reg &varying_offset,
>                                          uint32_t const_offset);
>  
>     bool run();
> @@ -399,9 +400,10 @@ public:
>     fs_reg fix_math_operand(fs_reg src);
>     fs_inst *emit_math(enum opcode op, fs_reg dst, fs_reg src0);
>     fs_inst *emit_math(enum opcode op, fs_reg dst, fs_reg src0, fs_reg src1);
> -   void emit_lrp(fs_reg dst, fs_reg x, fs_reg y, fs_reg a);
> -   void emit_minmax(uint32_t conditionalmod, fs_reg dst,
> -                    fs_reg src0, fs_reg src1);
> +   void emit_lrp(const fs_reg &dst, const fs_reg &x, const fs_reg &y,
> +                 const fs_reg &a);
> +   void emit_minmax(uint32_t conditionalmod, const fs_reg &dst,
> +                    const fs_reg &src0, const fs_reg &src1);
>     bool try_emit_saturate(ir_expression *ir);
>     bool try_emit_mad(ir_expression *ir, int mul_arg);
>     void try_replace_with_sel();
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 172ad6e..0c376a7 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -212,7 +212,8 @@ fs_visitor::visit(ir_dereference_array *ir)
>  }
>  
>  void
> -fs_visitor::emit_lrp(fs_reg dst, fs_reg x, fs_reg y, fs_reg a)
> +fs_visitor::emit_lrp(const fs_reg &dst, const fs_reg &x, const fs_reg &y,
> +                     const fs_reg &a)
>  {
>     if (brw->gen < 6 ||
>         !x.is_valid_3src() ||
> @@ -225,8 +226,9 @@ fs_visitor::emit_lrp(fs_reg dst, fs_reg x, fs_reg y, fs_reg a)
>  
>        emit(MUL(y_times_a, y, a));
>  
> -      a.negate = !a.negate;
> -      emit(ADD(one_minus_a, a, fs_reg(1.0f)));
> +      fs_reg negative_a = a;
> +      negative_a.negate = !a.negate;
> +      emit(ADD(one_minus_a, negative_a, fs_reg(1.0f)));

If you left the type of 'a' intact this change wouldn't be needed. But I don't
have preference whether letting the compiler to emit the copy or doing it
manually here. I like the change and either way you choose:

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>

>        emit(MUL(x_times_one_minus_a, x, one_minus_a));
>  
>        emit(ADD(dst, x_times_one_minus_a, y_times_a));
> @@ -239,8 +241,8 @@ fs_visitor::emit_lrp(fs_reg dst, fs_reg x, fs_reg y, fs_reg a)
>  }
>  
>  void
> -fs_visitor::emit_minmax(uint32_t conditionalmod, fs_reg dst,
> -                        fs_reg src0, fs_reg src1)
> +fs_visitor::emit_minmax(uint32_t conditionalmod, const fs_reg &dst,
> +                        const fs_reg &src0, const fs_reg &src1)
>  {
>     fs_inst *inst;
>  
> -- 
> 1.8.3.2
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev