[Mesa-dev] [PATCH 02/57] i965/vec4: Replace dst/src_reg::reg_offset with dst/src_reg::offset expressed in bytes.

[Iago Toral]

On Wed, 2016-09-07 at 18:48 -0700, Francisco Jerez wrote:
(...)
>  src/mesa/drivers/dri/i965/brw_ir_vec4.h            |  4 +-
>  src/mesa/drivers/dri/i965/brw_vec4.cpp             | 61
> ++++++++++++----------
>  .../drivers/dri/i965/brw_vec4_cmod_propagation.cpp |  2 +-
>  .../drivers/dri/i965/brw_vec4_copy_propagation.cpp |  4 +-
>  .../drivers/dri/i965/brw_vec4_live_variables.h     |  8 +--
>  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp         |  2 +-
>  .../drivers/dri/i965/brw_vec4_reg_allocate.cpp     |  4 +-
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp     | 14 ++---
>  8 files changed, 51 insertions(+), 48 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> index 3813bb8..4f49428 100644
> --- a/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> +++ b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> @@ -65,7 +65,7 @@ offset(src_reg reg, unsigned delta)
>  {
>     assert(delta == 0 ||
>            (reg.file != ARF && reg.file != FIXED_GRF && reg.file !=
> IMM));
> -   reg.reg_offset += delta;
> +   reg.offset += delta * (reg.file == UNIFORM ? 16 : REG_SIZE);
>     return reg;
>  }
>  
> @@ -134,7 +134,7 @@ offset(dst_reg reg, unsigned delta)
>  {
>     assert(delta == 0 ||
>            (reg.file != ARF && reg.file != FIXED_GRF && reg.file !=
> IMM));
> -   reg.reg_offset += delta;
> +   reg.offset += delta * (reg.file == UNIFORM ? 16 : REG_SIZE);
>     return reg;
>  }
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index d52fdc0..dd058db 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -68,7 +68,7 @@ src_reg::src_reg()
>  src_reg::src_reg(struct ::brw_reg reg) :
>     backend_reg(reg)
>  {
> -   this->reg_offset = 0;
> +   this->offset = 0;
>     this->reladdr = NULL;
>  }
>  
> @@ -125,7 +125,7 @@ dst_reg::dst_reg(enum brw_reg_file file, int nr,
> brw_reg_type type,
>  dst_reg::dst_reg(struct ::brw_reg reg) :
>     backend_reg(reg)
>  {
> -   this->reg_offset = 0;
> +   this->offset = 0;
>     this->reladdr = NULL;
>  }
>  
> @@ -395,7 +395,7 @@ vec4_visitor::opt_vector_float()
>            * sequence.  Combine anything we've accumulated so far.
>            */
>           if (last_reg != inst->dst.nr ||
> -             last_reg_offset != inst->dst.reg_offset ||
> +             last_reg_offset != inst->dst.offset / REG_SIZE ||
>               last_reg_file != inst->dst.file ||
>               (vf > 0 && dest_type != need_type)) {
>  
> @@ -439,7 +439,7 @@ vec4_visitor::opt_vector_float()
>              imm_inst[inst_count++] = inst;
>  
>              last_reg = inst->dst.nr;
> -            last_reg_offset = inst->dst.reg_offset;
> +            last_reg_offset = inst->dst.offset / REG_SIZE;
>              last_reg_file = inst->dst.file;
>              if (vf > 0)
>                 dest_type = need_type;
> @@ -539,8 +539,8 @@ vec4_visitor::split_uniform_registers()
>  
>  	 assert(!inst->src[i].reladdr);
>  
> -         inst->src[i].nr += inst->src[i].reg_offset;
> -	 inst->src[i].reg_offset = 0;
> +         inst->src[i].nr += inst->src[i].offset / 16;
> +	 inst->src[i].offset %= 16;
>        }
>     }
>  }
> @@ -857,7 +857,7 @@
> vec4_visitor::move_push_constants_to_pull_constants()
>  
>  	 inst->src[i].file = temp.file;
>           inst->src[i].nr = temp.nr;
> -	 inst->src[i].reg_offset = temp.reg_offset;
> +	 inst->src[i].offset %= 16;

So it seems that temp.offset is going to be 0 here and that's why
you're making this change. Looks good to me, just making sure that this
is not something unintended since it is not quite following the pattern
of directly translating the original code.

>  	 inst->src[i].reladdr = NULL;
>        }
>     }

(...)

> @@ -1831,7 +1834,7 @@ vec4_visitor::convert_to_hw_regs()
>           struct brw_reg reg;
>           switch (src.file) {
>           case VGRF:
> -            reg = brw_vec8_grf(src.nr + src.reg_offset, 0);
> +            reg = brw_vec8_grf(src.nr + src.offset / REG_SIZE, 0);
>              reg.type = src.type;
>              reg.swizzle = src.swizzle;
>              reg.abs = src.abs;
> @@ -1840,8 +1843,8 @@ vec4_visitor::convert_to_hw_regs()
>  
>           case UNIFORM:
>              reg = stride(brw_vec4_grf(prog_data-
> >base.dispatch_grf_start_reg +
> -                                      (src.nr + src.reg_offset) / 2,
> -                                      ((src.nr + src.reg_offset) %
> 2) * 4),
> +                                      (src.nr + src.offset / 4) / 2,
> +                                      ((src.nr + src.offset / 4) % 

Shouldn't we divide by 16 instead of 4 here?

> 2) * 4),
>                           0, 4, 1);
>              reg.type = src.type;
>              reg.swizzle = src.swizzle;
> @@ -1887,14 +1890,14 @@ vec4_visitor::convert_to_hw_regs()
>  
>        switch (inst->dst.file) {
>        case VGRF:
> -         reg = brw_vec8_grf(dst.nr + dst.reg_offset, 0);
> +         reg = brw_vec8_grf(dst.nr + dst.offset / REG_SIZE, 0);
>           reg.type = dst.type;
>           reg.writemask = dst.writemask;
>           break;
>