[Mesa-dev] [PATCH 1/7] i965: Use 4 bits to store nr_userclip in brw_clip.h.

[Eric Anholt]

On Mon, 26 Sep 2011 14:28:25 -0700, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On 09/25/2011 09:21 AM, Paul Berry wrote:
> > Since the i965 driver supports 8 clipping planes now, we need 4 bits
> > to store the number of user clipping planes, not 3.
> > 
> > In theory this isn't strictly necessary, since brw_clip.h is only used
> > on pre-GEN6, and pre-GEN6 only advertises support for 6 clipping
> > planes, but it seems wise to err on the safe side.
> > ---
> >  src/mesa/drivers/dri/i965/brw_clip.h |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_clip.h b/src/mesa/drivers/dri/i965/brw_clip.h
> > index 8647847..029270a 100644
> > --- a/src/mesa/drivers/dri/i965/brw_clip.h
> > +++ b/src/mesa/drivers/dri/i965/brw_clip.h
> > @@ -44,7 +44,7 @@
> >  struct brw_clip_prog_key {
> >     GLbitfield64 attrs;
> >     GLuint primitive:4;
> > -   GLuint nr_userclip:3;
> > +   GLuint nr_userclip:4;
> >     GLuint do_flat_shading:1;
> >     GLuint pv_first:1;
> >     GLuint do_unfilled:1;
> > @@ -55,7 +55,7 @@ struct brw_clip_prog_key {
> >     GLuint copy_bfc_cw:1;
> >     GLuint copy_bfc_ccw:1;
> >     GLuint clip_mode:3;
> > -   GLuint pad0:11;
> > +   GLuint pad0:10;
> >  
> >     GLfloat offset_factor;
> >     GLfloat offset_units;
> 
> pad0 is entirely pointless.  It makes sense to have these for hardware
> structures, but for a hash table key...?  The compiler can't be -that-
> dumb.  I'd just remove it.

It's the style of the code since day 1.  I think the idea was that you
had an explicit count of the bits remaining, so it would force you to
think about how many bits you need so that you don't pointlessly create
a new dword of hash key.

On the other hand, the pad counts have ended up out of sync in the past,
being miserly in bits has created serious, hard-to-find bugs, pahole
will tell you the truth easily, and I would give even odds on whether
the bit-shifting to reduce hash key size actually improves performance
or not anyway.

I'd be in favor of seeing the explicit pad bits die.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20110927/b3ff2aa3/attachment.pgp>