[Mesa-dev] [PATCH 27/59] i965/fs: add a stride helper

[Iago Toral]

On Mon, 2016-05-02 at 18:48 -0700, Francisco Jerez wrote:
> Samuel Iglesias Gonsálvez <siglesias at igalia.com> writes:
> 
> > From: Connor Abbott <connor.w.abbott at intel.com>
> >
> > Similar to retype() and offset().
> > ---
> >  src/mesa/drivers/dri/i965/brw_ir_fs.h | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_ir_fs.h b/src/mesa/drivers/dri/i965/brw_ir_fs.h
> > index e4f20f4..abda2c3 100644
> > --- a/src/mesa/drivers/dri/i965/brw_ir_fs.h
> > +++ b/src/mesa/drivers/dri/i965/brw_ir_fs.h
> > @@ -78,6 +78,14 @@ retype(fs_reg reg, enum brw_reg_type type)
> >  }
> >  
> >  static inline fs_reg
> > +stride(fs_reg reg, unsigned stride)
> > +{
> > +   if (reg.stride != 0)
> > +      reg.stride = stride;
> > +   return reg;
> > +}
> > +
> 
> This only works if reg.stride == 0 or 1, we need to honour the stride of
> the original register (e.g. by doing reg.stride *= stride) or you'll end
> up taking components not part of the region given as argument.  It gets
> messy with ARF and HW registers because they represent the stride
> differently...  I suggest that instead of fixing this you take the
> following patch from my SIMD32 branch that introduces a somewhat easier
> to use helper: Instead of doing 'stride(horiz_offset(retype(reg, t), i),
> type_sz(reg.type) / type_sz(t))' to take the i-th subcomponent of type t
> of the original register you would just do 'subscript(reg, t, i)'. 

Actually, I think this is not what we want in a number of places.

Sometimes we just want to apply a stride to a register, no type changes
or anything. That is, we just want to do something like stride(reg, 2).
Of course, we can code that with subscript as subscript(reg, retype(reg,
type_that is_half_the_size), 0) but that is certainly forced. For
example, we have code such as:

bld.MOV(tmp, stride(component_i, 2));
bld.MOV(horiz_offset(tmp, 8 * multiplier),
        stride(horiz_offset(component_i, 1), 2));

where component_i is of type F. Of course, we could retype component_i
to DF when we pass it to subscript, and use type F for the type
parameter, so that subscript does what we want, but it is certainly not
very natural. Subscript is all about accessing data with a different
type and we have some places where we want to do that and subscript does
what we want, but we also want to use a "normal" stride sometimes.

Another example, in lowering of PACK, we do something like this:

ibld.MOV(stride(horiz_offset(retype(dst, inst->src[i].type), i), 
                inst->sources),
         inst->src[i]);


Here the stride we want is not based on any relation between types (even
if I think in this case it would work because inst->sources will be 2
and the type of inst->src[i] will be half of the type of the dst).

I think that even if subscript can be useful in some of our cases, we
still want the stride helper for a bunch of other cases.

>  I
> think I've looked through all the uses of stride() in your branch and
> they are all sort of an open-coded version of subscript().  This will
> also address the issue Ken pointed out in patch 29 related to the use of
> horiz_offset() on uniforms (or anything with stride other than one,
> really) -- See the attached patch (yes, correct ARF/HW reg handling will
> be required for SIMD32...).
> 
> > +static inline fs_reg
> >  byte_offset(fs_reg reg, unsigned delta)
> >  {
> >     switch (reg.file) {
> > -- 
> > 2.5.0
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev