[Mesa-dev] [PATCH 04/57] i965/fs: Replace fs_reg::subreg_offset with fs_reg::offset expressed in bytes.

[Iago Toral]

On Wed, 2016-09-07 at 18:48 -0700, Francisco Jerez wrote:
> The fs_reg::subreg_offset and ::offset fields are now redundant, the
> sub-GRF offset can just be added to the single ::offset field
> expressed in byte units.  The current subreg_offset value can be
> recovered by applying the following rule: Replace each rvalue
> reference of subreg_offset like 'x = r.subreg_offset' with 'x =
> r.offset % reg_unit', and each lvalue reference like 'r.subreg_offset
> = x' with 'r.offset = ROUND_DOWN_TO(r.offset, reg_unit) + x'.
> For the same reason as in the previous patches, this doesn't attempt
> to be particularly clever about simplifying the result in the
> interest
> of keeping the rather lengthy patch as obvious as possible.  I'll
> come
> back later to clean up any ugliness introduced here.
> ---
(...)
> diff --git a/src/mesa/drivers/dri/i965/brw_ir_fs.h
> b/src/mesa/drivers/dri/i965/brw_ir_fs.h
> index 10da31e..19ef242 100644
> --- a/src/mesa/drivers/dri/i965/brw_ir_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_ir_fs.h
> @@ -52,12 +52,6 @@ public:
>     /** Smear a channel of the reg to all channels. */
>     fs_reg &set_smear(unsigned subreg);
>  
> -   /**
> -    * Offset in bytes from the start of the register.  Values up to
> a
> -    * backend_reg::reg_offset unit are valid.
> -    */
> -   int subreg_offset;
> -
>     /** Register region horizontal stride */
>     uint8_t stride;
>  };
> @@ -87,15 +81,15 @@ byte_offset(fs_reg reg, unsigned delta)
>     case ATTR:
>     case UNIFORM: {
>        const unsigned reg_size = (reg.file == UNIFORM ? 4 :
> REG_SIZE);
> -      const unsigned suboffset = reg.subreg_offset + delta;
> +      const unsigned suboffset = reg.offset % reg_size + delta;
>        reg.offset += ROUND_DOWN_TO(suboffset, reg_size);
> -      reg.subreg_offset = suboffset % reg_size;
> +      reg.offset = ROUND_DOWN_TO(reg.offset, reg_size) + suboffset %
> reg_size;
> 
I understand that you want to keep the idea of just making direct
translations for now to ease the review process, so this is fine. Of
course, with offset now being in byte units a byte_offset() helper for
these cases is really a trivial addition, so the above is rather
convoluted at this point in the series. This is all cleaned up later on
in patch 32.

> 
>        break;
>     }
>     case MRF: {
> -      const unsigned suboffset = reg.subreg_offset + delta;
> +      const unsigned suboffset = reg.offset % REG_SIZE + delta;
>        reg.nr += suboffset / REG_SIZE;
> -      reg.subreg_offset = suboffset % REG_SIZE;
> +      reg.offset = ROUND_DOWN_TO(reg.offset, REG_SIZE) + suboffset %
> REG_SIZE;
       break;
>     }
>     case ARF:
> @@ -193,7 +187,7 @@ static inline unsigned
>  reg_offset(const fs_reg &r)
>  {
>     return (r.file == VGRF || r.file == IMM ? 0 : r.nr) *
> -          (r.file == UNIFORM ? 4 : REG_SIZE) + r.offset +
> r.subreg_offset;
> +          (r.file == UNIFORM ? 4 : REG_SIZE) + r.offset;
>  }
>  
>  /**