[Mesa-dev] [PATCH 1/9] i965/fs: Store a pointer to brw_sampler_prog_key_data in the visitor.

[Jason Ekstrand]

I'll second Topi's comment.  Other than that, not much jumpped out at me.
I did make a comment on 7 which also applies (somewhat) to 8 and I made
another comment on 5.  However, none of this is blockers so this series is

Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>

On Wed, Mar 11, 2015 at 11:59 AM, Kenneth Graunke <kenneth at whitecape.org>
wrote:

> On Wednesday, March 11, 2015 10:44:26 AM Pohjolainen, Topi wrote:
> > On Mon, Mar 09, 2015 at 01:58:51AM -0700, Kenneth Graunke wrote:
> > > The NIR backend hardcodes brw_wm_prog_key at the moment, which won't
> > > work when we support scalar VS.  We could use get_tex(), but it's a
> > > static method.  I was going to promote it to fs_visitor, but then
> > > realized that both parameters (stage and key) are already members.
> > >
> > > It then occured to me that we could just set up a pointer in the
> > > constructor, and skip having a function altogether.
> > >
> > > This patch also converts all existing users to use key_tex.
> > >
> > > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> >
> > With the few nits:
> >
> > Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> >
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_fs.h           |  2 +
> > >  src/mesa/drivers/dri/i965/brw_fs_nir.cpp     |  3 +-
> > >  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 55
> ++++++++++++----------------
> > >  3 files changed, 27 insertions(+), 33 deletions(-)
> > >
> > > diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
> b/src/mesa/drivers/dri/i965/brw_fs.h
> > > index ee6ba98..7f4916a 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_fs.h
> > > +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> > > @@ -418,6 +418,8 @@ public:
> > >     void visit_atomic_counter_intrinsic(ir_call *ir);
> > >
> > >     const void *const key;
> > > +   struct brw_sampler_prog_key_data *key_tex;
> >
> > This could be constant pointer as well, we only use it for reading in the
> > visitor. (Also prevents the need to cast the constant 'key' pointer to
> > non-constant in init()).
>
> Good catch!  I've changed it to:
>
> const struct brw_sampler_prog_key_Data *key_tex;
>
> and changed the casts in init() to be "const struct brw_wm_prog_key" and
> so on.  I couldn't add the second 'const' (which prevents assignments to
> key_tex) since I can't really set this from a constructor initializer
> list - I need the switch statement in init().  But, this is probably the
> most important one.
>
> Also fixed the word wrapping.  Thanks!
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150311/13acd11c/attachment.html>