[Mesa-stable] [PATCH 2/3] i965/vec4: fix register width for DF VGRF and UNIFORM

[Samuel Iglesias Gonsálvez]

On Mon, 2017-05-01 at 14:55 +0200, Samuel Iglesias Gonsálvez wrote:
> El Viernes, 28 de abril de 2017 16:27:56 Francisco Jerez escribió:
> > Samuel Iglesias Gonsálvez <siglesias at igalia.com> writes:
> > > On gen7, the swizzles used in DF align16 instructions works for
> > > element
> > > size of 32 bits, so we can address only 2 consecutive DFs. As we
> > > assumed
> > > that in the rest of the code and prepare the instructions for
> > > this
> > > (scalarize_df()), we need to set it to two again.
> > > 
> > > However, for DF align1 instructions, a width of 2 is wrong as we
> > > are not
> > > reading the data we want. For example, an uniform would have a
> > > region of
> > > <0, 2, 1> so it would repeat the first 2 DFs, when we wanted to
> > > access
> > > to the first 4.
> > > 
> > > This patch sets the default one to 4 and then modifies the width
> > > of
> > > align16 instruction's DF sources when we translate the logical
> > > swizzle
> > > to the physical one.
> > > 
> > > Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> > > Cc: "17.1" <mesa-stable at lists.freedesktop.org>
> > > ---
> > > 
> > >  src/intel/compiler/brw_vec4.cpp | 13 ++++++++-----
> > >  1 file changed, 8 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/src/intel/compiler/brw_vec4.cpp
> > > b/src/intel/compiler/brw_vec4.cpp index 95f96ea69c0..8b755e1b75e
> > > 100644
> > > --- a/src/intel/compiler/brw_vec4.cpp
> > > +++ b/src/intel/compiler/brw_vec4.cpp
> > > @@ -2003,9 +2003,7 @@ vec4_visitor::convert_to_hw_regs()
> > > 
> > >           struct brw_reg reg;
> > >           switch (src.file) {
> > >           case VGRF: {
> > > 
> > > -            const unsigned type_size = type_sz(src.type);
> > > -            const unsigned width = REG_SIZE / 2 / MAX2(4,
> > > type_size);
> > > -            reg = byte_offset(brw_vecn_grf(width, src.nr, 0),
> > > src.offset);
> > > +            reg = byte_offset(brw_vecn_grf(4, src.nr, 0),
> > > src.offset);
> > > 
> > >              reg.type = src.type;
> > >              reg.abs = src.abs;
> > >              reg.negate = src.negate;
> > > 
> > > @@ -2013,12 +2011,11 @@ vec4_visitor::convert_to_hw_regs()
> > > 
> > >           }
> > >           
> > >           case UNIFORM: {
> > > 
> > > -            const unsigned width = REG_SIZE / 2 / MAX2(4,
> > > type_sz(src.type));> 
> > >              reg = stride(byte_offset(brw_vec4_grf(
> > >              
> > >                                          prog_data-
> > > >base.dispatch_grf_star
> > >                                          t_reg +
> > >                                          src.nr / 2, src.nr % 2 *
> > > 4),
> > >                                       
> > >                                       src.offset),
> > > 
> > > -                         0, width, 1);
> > > +                         0, 4, 1);
> > > 
> > >              reg.type = src.type;
> > >              reg.abs = src.abs;
> > >              reg.negate = src.negate;
> > > 
> > > @@ -2576,6 +2573,12 @@ vec4_visitor::apply_logical_swizzle(struct
> > > brw_reg
> > > *hw_reg,> 
> > >     assert(brw_is_single_value_swizzle(reg.swizzle) ||
> > >     
> > >            is_supported_64bit_region(inst, arg));
> > > 
> > > +   /* Apply the region <2, 2, 1> for GRF or <0, 2, 1> for
> > > uniforms, as
> > > align16 +    * HW can only do 32-bit swizzle channels.
> > > +    */
> > > +   if (reg.file == UNIFORM || reg.file == VGRF)
> > > +      hw_reg->width = BRW_WIDTH_2;
> > 
> > Any reason this is conditional on the register file?  Originally we
> > were
> > only setting the width to 2 for the UNIFORM and VGRF files, but
> > that was
> > probably an oversight...
> > 
> 
> No reason, this was an oversight. I have removed locally the
> conditional.
> 
> Do you get your R-b to it then?
> 

s/get/give

> Sam
> 
> > > +
> > > 
> > >     if (is_supported_64bit_region(inst, arg) &&
> > >     
> > >         !is_gen7_supported_64bit_swizzle(inst, arg)) {
> > >        
> > >        /* Supported 64-bit swizzles are those such that their
> > > first two
> > > 
> > > _______________________________________________
> > > mesa-stable mailing list
> > > mesa-stable at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-stable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-stable/attachments/20170502/323ba587/attachment.sig>