On Dec 22, 2014 2:49 PM, "Ian Romanick" <i...@freedesktop.org> 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 <jason.ekstr...@intel.com
> > <mailto:jason.ekstr...@intel.com>>
> >
> > On Fri, Dec 19, 2014 at 2:20 PM, Ian Romanick <i...@freedesktop.org
> > <mailto:i...@freedesktop.org>> wrote:
> >
> > From: Ian Romanick <ian.d.roman...@intel.com
> > <mailto:ian.d.roman...@intel.com>>
> >
> > 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 <ian.d.roman...@intel.com
> > <mailto:ian.d.roman...@intel.com>>
> > ---
> > 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
> > mesa-dev@lists.freedesktop.org <mailto:
mesa-dev@lists.freedesktop.org>
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
