[Mesa-dev] [PATCH 05/12] i965/fs: Use offset a lot more places

[Kenneth Graunke]

On Tuesday, September 23, 2014 10:50:08 AM Matt Turner wrote:
> On Mon, Sep 22, 2014 at 1:42 PM, Matt Turner <mattst88 at gmail.com> wrote:
> > On Mon, Sep 22, 2014 at 1:40 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> >> On Fri, Sep 19, 2014 at 5:34 PM, Matt Turner <mattst88 at gmail.com> wrote:
> >>> On Fri, Sep 19, 2014 at 1:10 PM, Jason Ekstrand <jason at jlekstrand.net>
> >>> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> >>> > b/src/mesa/drivers/dri/i965/brw_fs.cpp
> >>> > index af8c087..ea91705 100644
> >>> > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> >>> > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> >>> > @@ -310,8 +310,8 @@ fs_visitor::VARYING_PULL_CONSTANT_LOAD(const fs_reg
> >>> > &dst,
> >>> >           inst->mlen = 1 + dispatch_width / 8;
> >>> >     }
> >>> >
> >>> > -   vec4_result.reg_offset += (const_offset & 3) * scale;
> >>> > -   instructions.push_tail(MOV(dst, vec4_result));
> >>> > +   fs_reg result = offset(vec4_result, (const_offset & 3) * scale);
> >>> > +   instructions.push_tail(MOV(dst, result));
> >>>
> >>> Isn't this going to cause us to copy an fs_reg twice, rather than just
> >>> setting .reg_offset?
> >>>
> >>> I'd like to check the generated code.
> >>
> >> What's your concern there?  Just that we're useing more CPU?
> >
> > Yeah, that we're now potentially copying an fs_reg twice when our
> > purpose is just to set a single integer.
> 
> Ignore this feedback for now. You've got a bunch of patches that would
> have to be rebased if we changed this and it should be really trivial
> to fix it up after the fact.

Have you /seen/ this function?  We're returning a blooming exec_list.  It's already nuts :)  I agree with cleaning it up later.

--Ken
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140923/1875ced9/attachment-0001.html>
-------------- 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/20140923/1875ced9/attachment-0001.sig>