[Mesa-dev] [PATCH 1/2] i965/fs: Use a const reference in fs_reg::equals instead of a pointer.

[Ian Romanick]

On 05/10/2012 04:10 PM, Kenneth Graunke wrote:
> This lets you omit some ampersands and is more idiomatic C++.  Using
> const also marks the function as not altering either register (which
> was obvious, but nice to enforce).

I'm increasingly *not* a fan of this sort of weird infix notation. 
Infix notation really only "reads" if operator symbols are used.  I'd 
much rather have a friend function.  Instead of

     if (foo.equals(bar)) {
     }

I'd much rather see

     if (equal(foo, bar)) {
     }

But that's just this programmers opinion.

Since you're keeping the style of the code as it existed,

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> Signed-off-by: Kenneth Graunke<kenneth at whitecape.org>
> ---
>   src/mesa/drivers/dri/i965/brw_fs.cpp         |    2 +-
>   src/mesa/drivers/dri/i965/brw_fs.h           |   28 +++++++++++++-------------
>   src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |    2 +-
>   3 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index fd67318..bcdedf0 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -1618,7 +1618,7 @@ fs_visitor::get_instruction_generating_reg(fs_inst *start,
>          end->predicated ||
>          end->force_uncompressed ||
>          end->force_sechalf ||
> -       !reg.equals(&end->dst)) {
> +       !reg.equals(end->dst)) {
>         return NULL;
>      } else {
>         return end;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index 6b45c4e..d4473a2 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -126,18 +126,18 @@ public:
>      fs_reg(enum register_file file, int reg, uint32_t type);
>      fs_reg(class fs_visitor *v, const struct glsl_type *type);
>
> -   bool equals(fs_reg *r)
> +   bool equals(const fs_reg&r) const
>      {
> -      return (file == r->file&&
> -	      reg == r->reg&&
> -	      reg_offset == r->reg_offset&&
> -	      type == r->type&&
> -	      negate == r->negate&&
> -	      abs == r->abs&&
> -	      memcmp(&fixed_hw_reg,&r->fixed_hw_reg,
> +      return (file == r.file&&
> +	      reg == r.reg&&
> +	      reg_offset == r.reg_offset&&
> +	      type == r.type&&
> +	      negate == r.negate&&
> +	      abs == r.abs&&
> +	      memcmp(&fixed_hw_reg,&r.fixed_hw_reg,
>   		     sizeof(fixed_hw_reg)) == 0&&
> -	      smear == r->smear&&
> -	      imm.u == r->imm.u);
> +	      smear == r.smear&&
> +	      imm.u == r.imm.u);
>      }
>
>      /** Register file: ARF, GRF, MRF, IMM. */
> @@ -291,10 +291,10 @@ public:
>      bool equals(fs_inst *inst)
>      {
>         return (opcode == inst->opcode&&
> -	      dst.equals(&inst->dst)&&
> -	      src[0].equals(&inst->src[0])&&
> -	      src[1].equals(&inst->src[1])&&
> -	      src[2].equals(&inst->src[2])&&
> +	      dst.equals(inst->dst)&&
> +	      src[0].equals(inst->src[0])&&
> +	      src[1].equals(inst->src[1])&&
> +	      src[2].equals(inst->src[2])&&
>   	saturate == inst->saturate&&
>   	predicated == inst->predicated&&
>   	conditional_mod == inst->conditional_mod&&
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 20d4c53..bb0c8bd 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -569,7 +569,7 @@ fs_visitor::emit_assignment_writes(fs_reg&l, fs_reg&r,
>   	 l.type = brw_type_for_base_type(type);
>   	 r.type = brw_type_for_base_type(type);
>
> -	 if (predicated || !l.equals(&r)) {
> +	 if (predicated || !l.equals(r)) {
>   	    fs_inst *inst = emit(BRW_OPCODE_MOV, l, r);
>   	    inst->predicated = predicated;
>   	 }