[Mesa-dev] [PATCH 04/17] i965/fs: Explicitly set the exec_size on the add(32) in interpolation setup

[Iago Toral]

On Fri, 2015-06-19 at 07:01 -0700, Jason Ekstrand wrote:
> 
> On Jun 19, 2015 5:09 AM, "Iago Toral" <itoral at igalia.com> wrote:
> >
> > On Thu, 2015-06-18 at 17:50 -0700, Jason Ekstrand wrote:
> > > Soon we will start using the builder to explicitly set all the
> execution
> > > sizes.  We could make a 32-wide builder, but the builder asserts
> that we
> > > never grow it which is usually a reasonable assumption.  Sinc this
> one
> > > instruction is a bit of an odd-ball, we just set the exec_size
> explicitly.
> >
> > So if I understand it right, the only point of this change is making
> > explicit that this instruction has a different execution size to
> ensure
> > that we notice it when we rewrite the code to set explicit execution
> > sizes with the new builder, right?
> 
> No, it's more that there is no good way to set it to SIMD32 with the
> builder because changing dispatch width in the builder can only go
> down and not up.

Aha, I see now, thanks.

>   In retrospect, I should have explicitly created the fs_inst rather
> than using the builder to emit it 16-wide and changing it later.

Yeah, I guess that would've made more sense, not a big deal anyway:

Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

> The reason this patch can stand on it's own is because, at this point
> in the series, the builder still uses the exec size guessing based on
> register widths.
> 
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 9 +++++----
> > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> > > index 4770838..b00825e 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> > > +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> > > @@ -1357,10 +1357,11 @@
> fs_visitor::emit_interpolation_setup_gen6()
> > >         */
> > >        fs_reg int_pixel_xy(GRF, alloc.allocate(dispatch_width /
> 8),
> > >                            BRW_REGISTER_TYPE_UW, dispatch_width *
> 2);
> > > -      abld.exec_all()
> > > -          .ADD(int_pixel_xy,
> > > -               fs_reg(stride(suboffset(g1_uw, 4), 1, 4, 0)),
> > > -               fs_reg(brw_imm_v(0x11001010)));
> > > +      fs_inst *add = abld.exec_all()
> > > +                         .ADD(int_pixel_xy,
> > > +                              fs_reg(stride(suboffset(g1_uw, 4),
> 1, 4, 0)),
> > > +                              fs_reg(brw_imm_v(0x11001010)));
> > > +      add->exec_size = dispatch_width * 2;
> > >
> > >        this->pixel_x = vgrf(glsl_type::float_type);
> > >        this->pixel_y = vgrf(glsl_type::float_type);
> >
> >
> 
>