[Mesa-dev] [PATCH 0/9] Skip automatic execsize for instructions with a width of 4

[Iago Toral]

On Wed, 2016-03-09 at 09:32 +0200, Pohjolainen, Topi wrote:
> On Mon, Mar 07, 2016 at 10:48:49AM +0100, Samuel Iglesias Gons?lvez wrote:
> > Hello,
> > 
> > There is only one patch from this series that has been reviewed (patch
> > 1).
> > 
> > Our plans is to start sending patches for adding fp64 support to i965
> > driver in the coming weeks but they depend on these patches.
> > 
> > Can someone take a look at them? ;)
> > 
> > Sam
> > 
> > 
> > On Thu, 2015-12-17 at 14:44 +0100, Samuel Iglesias Gonsálvez wrote:
> > > Hello,
> > > 
> > > This patch series is a updated version of the one Iago sent last
> > > week [0] that includes patches for gen6 too, as suggested by Jason.
> > > 
> > > We checked the gen9 code paths that work with a horizontal width of 4
> > > and we think there won't be any regression on gen9... but we don't
> > > have any gen9 machine to run piglit with these patches. Can someone
> > > check it?
> > > 
> > > Please read the original cover letter [0] for more information.
> > > 
> > > Sam
> > > 
> > > [0] http://lists.freedesktop.org/archives/mesa-dev/2015-December/1027
> > > 46.html
> > > 
> > > Iago Toral Quiroga (5):
> > >   i965/eu: set correct execution size in brw_NOP
> > >   i965/fs: set execution size for SEND messages in
> > >     generate_uniform_pull_constant_load_gen7
> 
> Then about the other change. I like it being explicitly set instead of just
> inheriting the size from the previous instruction.
> 
> @@ -1248,6 +1248,7 @@ fs_generator::generate_uniform_pull_constant_load_gen7(fs_inst *inst,
>        brw_set_default_compression_control(p, BRW_COMPRESSION_NONE);
>        brw_set_default_mask_control(p, BRW_MASK_DISABLE);
>        brw_inst *send = brw_next_insn(p, BRW_OPCODE_SEND);
> +      brw_inst_set_exec_size(devinfo, send, dst.width);
> 
> But I'm seeing other occurrences of BRW_OPCODE_SEND as well. For example, there
> are such instructions generated in generate_urb_read/write() which are not 
> addressed. Don't we end up there with doubles as well needing the same
> treatment?

Probably those other SENDs always operate with a width of 8/16 (the
default) and don't need to be fixed for other cases because they don't
happen. Notice that we are only trying to fix the cases of width = 4
here, which are the conflicting ones, so if some paths never execute
instructions with a width of 4 we don't need to do anything about them.

What we have done to identify the cases that need fixing was adding this
assertion in brw_set_dest (brw_eu_emit.c):

   assert(dest.width != BRW_EXECUTE_4 ||
          brw_inst_exec_size(devinfo, inst) == dest.width);

That catches any instruction with a width of 4 (the ones that need
fixing) that does not have the correct execsize set. We ran this through
piglit and dEQP functional's tests for all the generations we mentioned
and fixed the cases that broke this assertion one by one.

Iago