[Mesa-dev] [PATCH] i965: Drop LOAD_PAYLOAD workaround in fs_visitor::emit_urb_writes().

[Kenneth Graunke]

On Saturday, May 30, 2015 08:11:40 AM Jason Ekstrand wrote:
> On Sat, May 30, 2015 at 12:34 AM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> > Now that Jason's LOAD_PAYLOAD improvements have landed, we don't need
> > this.  Passing 1 for the number of header registers already takes care
> > of setting force_writemask_all on the header copy.
> >
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > ---
> >  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 16 ++++------------
> >  1 file changed, 4 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> > index e336b73..c9a2644 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> > @@ -1980,20 +1980,12 @@ fs_visitor::emit_urb_writes()
> >           fs_reg *payload_sources = ralloc_array(mem_ctx, fs_reg, length + 1);
> >           fs_reg payload = fs_reg(GRF, alloc.allocate(length + 1),
> >                                   BRW_REGISTER_TYPE_F, dispatch_width);
> > -
> > -         /* We need WE_all on the MOV for the message header (the URB handles)
> > -          * so do a MOV to a dummy register and set force_writemask_all on the
> > -          * MOV.  LOAD_PAYLOAD will preserve that.
> > -          */
> 
> Yeah, that's terrible.  Thanks for cleaning this up.

Sure.  I was in the area (making things more terrible) :D

> > -         fs_reg dummy = fs_reg(GRF, alloc.allocate(1),
> > -                               BRW_REGISTER_TYPE_UD);
> > -         fs_inst *inst = emit(MOV(dummy, fs_reg(retype(brw_vec8_grf(1, 0),
> > -                                                       BRW_REGISTER_TYPE_UD))));
> > -         inst->force_writemask_all = true;
> > -         payload_sources[0] = dummy;
> > +         payload_sources[0] =
> > +            fs_reg(retype(brw_vec8_grf(1, 0), BRW_REGISTER_TYPE_UD));
> >
> >           memcpy(&payload_sources[1], sources, length * sizeof sources[0]);
> > -         emit(LOAD_PAYLOAD(payload, payload_sources, length + 1, 1));
> > +         fs_inst *inst =
> > +            emit(LOAD_PAYLOAD(payload, payload_sources, length + 1, 1));
> 
> There's no need to save the return value here.  We just throw it away
> one line later.  Let's move the fs_inst * declaration to the URB_WRITE
> below.  With that,
> 
> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

Oops.  Yeah, I originally deleted the declaration, then went "it doesn't
compile!" and put it back on some random instruction.  Putting it on the
URB_WRITE is clearly best :)

Thanks!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150530/9cc91041/attachment.sig>