On Tue, 2016-08-23 at 11:21 +0200, Michael Schellenberger Costa wrote:
> Hi Iago,
>
> given that the idea here was to unify vec4 and fs you might want to
> adopt the names/function types accordingly.
>
> In brw_ir_fs.h there is byte_offset that returns a fs_reg while you
> have
> void add_byte_offset.
Yeah, the reason for that is that in the FS backend we only have
fs_reg, so byte_offset takes that as argument and returns an updated
copy of an fs_reg that we can return directly from the offset() helper.
In the vec4 backend we have to deal with src_reg and dst_reg but I was
trying not want to duplicate the logic for each (the switch statement).
I did not notice that the FS backend was using byte_offset() directly
in other places, so it might be a good idea to make a byte_offset()
helper in the vec4 backend that works in the same fashion. That means
that I'll move the switch statement to another helper that takes a
backend_reg as input parameter and call that from byte_offset.
Iago
> --Michael
>
> Am 23.08.2016 um 10:24 schrieb Iago Toral Quiroga:
> >
> > This will make it more consistent with the FS implementation of the
> > same
> > helper and will provide more flexibility that will come in handy,
> > for
> > example, when we add a SIMD lowering pass in the vec4 backend.
> >
> > v2:
> > - Move the switch statement to add_byte_offset (Iago)
> > - Remove the assert on the register file, it is redundant with the
> > switch
> > (Michael Schellenberger)
> > - Fix use of '=' instead of '==' (Michael Schellenberger,
> > Francesco Ansanelli)
> > ---
> > src/mesa/drivers/dri/i965/brw_ir_vec4.h | 36
> > +++++++++++++++++++++++++--------
> > 1 file changed, 28 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> > b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> > index 81b6a13..d55b522 100644
> > --- a/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> > +++ b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> > @@ -60,12 +60,33 @@ retype(src_reg reg, enum brw_reg_type type)
> > return reg;
> > }
> >
> > +static inline void
> > +add_byte_offset(backend_reg *reg, unsigned delta)
> > +{
> > + switch (reg->file) {
> > + case BAD_FILE:
> > + break;
> > + case MRF:
> > + case VGRF:
> > + case ATTR:
> > + case UNIFORM: {
> > + const unsigned suboffset = reg->subreg_offset + delta;
> > + reg->reg_offset += suboffset / REG_SIZE;
> > + reg->subreg_offset += suboffset % REG_SIZE;
> > + /* Align16 requires that register accesses are 16-byte
> > aligned */
> > + assert(reg->subreg_offset % 16 == 0);
> > + break;
> > + }
> > + default:
> > + assert(delta == 0);
> > + }
> > +}
> > +
> > static inline src_reg
> > -offset(src_reg reg, unsigned delta)
> > +offset(src_reg reg, unsigned width, unsigned delta)
> > {
> > - assert(delta == 0 ||
> > - (reg.file != ARF && reg.file != FIXED_GRF && reg.file !=
> > IMM));
> > - reg.reg_offset += delta;
> > + unsigned byte_offset = delta * width * type_sz(reg.type);
> > + add_byte_offset(®, byte_offset);
> > return reg;
> > }
> >
> > @@ -130,11 +151,10 @@ retype(dst_reg reg, enum brw_reg_type type)
> > }
> >
> > static inline dst_reg
> > -offset(dst_reg reg, unsigned delta)
> > +offset(dst_reg reg, unsigned width, unsigned delta)
> > {
> > - assert(delta == 0 ||
> > - (reg.file != ARF && reg.file != FIXED_GRF && reg.file !=
> > IMM));
> > - reg.reg_offset += delta;
> > + unsigned byte_offset = delta * width * type_sz(reg.type);
> > + add_byte_offset(®, byte_offset);
> > return reg;
> > }
> >
> >
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
