[Mesa-dev] [PATCH] intel/fs: Get rid of fs_inst::equals

[Iago Toral]

On Fri, 2019-01-25 at 15:47 -0600, Jason Ekstrand wrote:
> There are piles of fields that it doesn't check so using it is a lie.
> The only reason why it's not causing problem is because it has
> exactly
> one user which only uses it for MOV instructions (which aren't very
> interesting) and only on Sandy Bridge and earlier hardware.  Just get
> rid of it and inline it in the one place that it's actually used.
> 
> Cc: Iago Toral Quiroga <itoral at igalia.com>
> ---
>  src/intel/compiler/brw_fs.cpp  | 29 +++++++----------------------
>  src/intel/compiler/brw_ir_fs.h |  1 -
>  2 files changed, 7 insertions(+), 23 deletions(-)
> 
> diff --git a/src/intel/compiler/brw_fs.cpp
> b/src/intel/compiler/brw_fs.cpp
> index 0359eb079f7..6b66116f46b 100644
> --- a/src/intel/compiler/brw_fs.cpp
> +++ b/src/intel/compiler/brw_fs.cpp
> @@ -212,27 +212,6 @@ fs_visitor::DEP_RESOLVE_MOV(const fs_builder
> &bld, int grf)
>     ubld.MOV(ubld.null_reg_f(), fs_reg(VGRF, grf,
> BRW_REGISTER_TYPE_F));
>  }
>  
> -bool
> -fs_inst::equals(fs_inst *inst) const
> -{
> -   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]) &&
> -           saturate == inst->saturate &&
> -           predicate == inst->predicate &&
> -           conditional_mod == inst->conditional_mod &&
> -           mlen == inst->mlen &&
> -           base_mrf == inst->base_mrf &&
> -           target == inst->target &&
> -           eot == inst->eot &&
> -           header_size == inst->header_size &&
> -           shadow_compare == inst->shadow_compare &&
> -           exec_size == inst->exec_size &&
> -           offset == inst->offset);
> -}
> -
>  bool
>  fs_inst::is_send_from_grf() const
>  {
> @@ -3410,7 +3389,13 @@ fs_visitor::remove_duplicate_mrf_writes()
>        if (inst->opcode == BRW_OPCODE_MOV &&
>  	  inst->dst.file == MRF) {
>           fs_inst *prev_inst = last_mrf_move[inst->dst.nr];
> -	 if (prev_inst && inst->equals(prev_inst)) {
> +	 if (prev_inst && prev_inst->opcode == BRW_OPCODE_MOV &&
> +             inst->dst.equals(prev_inst->dst) &&
> +	     inst->src[0].equals(prev_inst->src[0]) &&

Wrong indentation. Otherwise:

Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>


> +             inst->saturate == prev_inst->saturate &&
> +             inst->predicate == prev_inst->predicate &&
> +             inst->conditional_mod == prev_inst->conditional_mod &&
> +             inst->exec_size == prev_inst->exec_size) {
>  	    inst->remove(block);
>  	    progress = true;
>  	    continue;
> diff --git a/src/intel/compiler/brw_ir_fs.h
> b/src/intel/compiler/brw_ir_fs.h
> index 08e3d83d910..d05357e822e 100644
> --- a/src/intel/compiler/brw_ir_fs.h
> +++ b/src/intel/compiler/brw_ir_fs.h
> @@ -347,7 +347,6 @@ public:
>  
>     void resize_sources(uint8_t num_sources);
>  
> -   bool equals(fs_inst *inst) const;
>     bool is_send_from_grf() const;
>     bool is_partial_write() const;
>     bool is_copy_payload(const brw::simple_allocator &grf_alloc)
> const;