On Dec 22, 2014 2:49 PM, "Ian Romanick" <<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a>> wrote: > > On 12/19/2014 06:20 PM, Jason Ekstrand wrote: > > I wrote basically this same patch when I was trying to speed up > > basically the same test. Unfortunately, I wasn't able to get much of an > > improvement out of it. Other than the one nit below, > > Reviewed-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a> > > <mailto:<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>>> > > > > On Fri, Dec 19, 2014 at 2:20 PM, Ian Romanick <<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a> > > <mailto:<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a>>> wrote: > > > > From: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a> > > <mailto:<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>>> > > > > Instead of having an extra pointer indirection in one of the hottest > > loops in the driver. > > > > On Bay Trail-D using Fedora 20 compile flags (-m64 -O2 -mtune=generic > > for 64-bit and -m32 -march=i686 -mtune=atom for 32-bit), affects > > Gl32Batch7: > > > > 32-bit: Difference at 95.0% confidence 1.98515% +/- 0.20814% (n=40) > > 64-bit: Difference at 95.0% confidence 1.5163% +/- 0.811016% (n=60) > > > > Signed-off-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a> > > <mailto:<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>>> > > --- > > src/mesa/drivers/dri/i965/brw_context.h | 2 +- > > src/mesa/drivers/dri/i965/brw_state_upload.c | 19 ++++++++++++++++--- > > 2 files changed, 17 insertions(+), 4 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_context.h > > b/src/mesa/drivers/dri/i965/brw_context.h > > index 92eb022..5f5f807 100644 > > --- a/src/mesa/drivers/dri/i965/brw_context.h > > +++ b/src/mesa/drivers/dri/i965/brw_context.h > > @@ -1384,7 +1384,7 @@ struct brw_context > > } perfmon; > > > > int num_atoms; > > - const struct brw_tracked_state **atoms; > > + const struct brw_tracked_state atoms[64]; > > > > > > Maybe a #define? > > Hmm... What I really wanted was to somehow use ARRAY_SIZE from the other > arrays. Since the define would only be used in one place, I don't think > it's that helpful. If you really care, you could do a bunch of MAX2 of ARRAY_SIZE and it should evaluate as a constant expression. I don't care that much. > > > /* If (INTEL_DEBUG & DEBUG_BATCH) */ > > struct { > > diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c > > b/src/mesa/drivers/dri/i965/brw_state_upload.c > > index a579781..9a5d49a 100644 > > --- a/src/mesa/drivers/dri/i965/brw_state_upload.c > > +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c > > @@ -357,6 +357,11 @@ void brw_init_state( struct brw_context *brw ) > > const struct brw_tracked_state **atoms; > > int num_atoms; > > > > + STATIC_ASSERT(ARRAY_SIZE(gen4_atoms) <= ARRAY_SIZE(brw->atoms)); > > + STATIC_ASSERT(ARRAY_SIZE(gen6_atoms) <= ARRAY_SIZE(brw->atoms)); > > + STATIC_ASSERT(ARRAY_SIZE(gen7_atoms) <= ARRAY_SIZE(brw->atoms)); > > + STATIC_ASSERT(ARRAY_SIZE(gen8_atoms) <= ARRAY_SIZE(brw->atoms)); > > + > > brw_init_caches(brw); > > > > if (brw->gen >= 8) { > > @@ -373,9 +378,17 @@ void brw_init_state( struct brw_context *brw ) > > num_atoms = ARRAY_SIZE(gen4_atoms); > > } > > > > - brw->atoms = atoms; > > brw->num_atoms = num_atoms; > > > > + /* This is to work around brw_context::atoms being declared > > const. We want > > + * it to be const, but it needs to be initialized somehow! > > + */ > > + struct brw_tracked_state *context_atoms = > > + (struct brw_tracked_state *) &brw->atoms[0]; > > + > > + for (int i = 0; i < num_atoms; i++) > > + context_atoms[i] = *atoms[i]; > > + > > while (num_atoms--) { > > assert((*atoms)->dirty.mesa | (*atoms)->dirty.brw); > > assert((*atoms)->emit); > > @@ -607,7 +620,7 @@ void brw_upload_state(struct brw_context *brw) > > prev = *state; > > > > for (i = 0; i < brw->num_atoms; i++) { > > - const struct brw_tracked_state *atom = brw->atoms[i]; > > + const struct brw_tracked_state *atom = &brw->atoms[i]; > > struct brw_state_flags generated; > > > > if (check_state(state, &atom->dirty)) { > > @@ -627,7 +640,7 @@ void brw_upload_state(struct brw_context *brw) > > } > > else { > > for (i = 0; i < brw->num_atoms; i++) { > > - const struct brw_tracked_state *atom = brw->atoms[i]; > > + const struct brw_tracked_state *atom = &brw->atoms[i]; > > > > if (check_state(state, &atom->dirty)) { > > atom->emit(brw); > > -- > > 1.8.1.4 > > > > _______________________________________________ > > mesa-dev mailing list > > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <mailto:<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a>> > > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > > >