[Mesa-dev] [PATCH 3/8] i965: Store the atoms directly in the context

[Ian Romanick]

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.ekstrand at intel.com
> <mailto:jason.ekstrand at intel.com>>
> 
> On Fri, Dec 19, 2014 at 2:20 PM, Ian Romanick <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
> 
>     From: Ian Romanick <ian.d.romanick at intel.com
>     <mailto:ian.d.romanick at 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.romanick at intel.com
>     <mailto:ian.d.romanick at 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 (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 at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>     http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>