On Wed, 2016-03-09 at 09:26 +0200, Pohjolainen, Topi wrote:
> On Wed, Mar 09, 2016 at 09:07:44AM +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
> > 
> > I don't have the series in my mailbox anymore, so I'll comment here. There 
> > is:
> > 
> >        brw_set_dest(p, send, dst);
> > @@ -1279,6 +1280,7 @@ 
> > fs_generator::generate_uniform_pull_constant_load_gen7(fs_inst *inst,
> >        /* dst = send(payload, a0.0 | <descriptor>) */
> >        brw_inst *insn = brw_send_indirect_message(
> >           p, BRW_SFID_SAMPLER, dst, src, addr);
> > +      brw_inst_set_exec_size(devinfo, insn, dst.width);
> > 
> > I wonder if we should modify brw_send_indirect_message() instead? It already
> > calls brw_inst_set_exec_size() itself:
> > 
> >    if (dst.width < BRW_EXECUTE_8)
> >       brw_inst_set_exec_size(devinfo, send, dst.width);
> 
> Actually you set this yourself in the next patch of the series. Is the
> previous in the caller side really needed after this?

Good catch! I'll give it a quick test through piglit with the assertion
I mentioned in my previous reply to see if we can drop this hunk, that's
the only way to be certain :)

Iago

> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to