[Mesa-dev] [PATCH 12/88] i965: add initial implementation of on disk shader cache

Timothy Arceri timothy.arceri at collabora.com
Fri Sep 30 04:32:51 UTC 2016


On Sun, 2016-09-25 at 20:38 -0700, Kenneth Graunke wrote:
> On Monday, September 26, 2016 1:28:35 PM PDT Timothy Arceri wrote:
> > 
> > On Sun, 2016-09-25 at 19:43 -0700, Kenneth Graunke wrote:
> > > 
> > > On Saturday, September 24, 2016 3:24:53 PM PDT Timothy Arceri
> > > wrote:
> > > > 
> > > > 
> > > > This uses the recently-added cache.c to write out the final
> > > > linked
> > > > binary for vertex and fragment shader programs.
> > > > 
> > > > This is based off the initial implementation done by Carl.
> > > > ---
> > > >  src/mesa/drivers/dri/i965/Makefile.sources   |   1 +
> > > >  src/mesa/drivers/dri/i965/brw_shader_cache.c | 390
> > > > +++++++++++++++++++++++++++
> > > >  src/mesa/drivers/dri/i965/brw_state.h        |   7 +
> > > >  3 files changed, 398 insertions(+)
> > > >  create mode 100644
> > > > src/mesa/drivers/dri/i965/brw_shader_cache.c
> > > > 
> > > > diff --git a/src/mesa/drivers/dri/i965/Makefile.sources
> > > > b/src/mesa/drivers/dri/i965/Makefile.sources
> > > > index df90cb4..bd2bd37 100644
> > > > --- a/src/mesa/drivers/dri/i965/Makefile.sources
> > > > +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> > > > @@ -147,6 +147,7 @@ i965_FILES = \
> > > >  	brw_sf_emit.c \
> > > >  	brw_sf.h \
> > > >  	brw_sf_state.c \
> > > > +	brw_shader_cache.cpp \
> > > >  	brw_state_batch.c \
> > > >  	brw_state_cache.c \
> > > >  	brw_state_dump.c \
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_shader_cache.c
> > > > b/src/mesa/drivers/dri/i965/brw_shader_cache.c
> > > > new file mode 100644
> > > > index 0000000..aba45b6
> > > > --- /dev/null
> > > > +++ b/src/mesa/drivers/dri/i965/brw_shader_cache.c
> > > > @@ -0,0 +1,390 @@
> > > > +/*
> > > > + * Copyright © 2014 Intel Corporation
> > > > + *
> > > > + * Permission is hereby granted, free of charge, to any person
> > > > obtaining a
> > > > + * copy of this software and associated documentation files
> > > > (the
> > > > "Software"),
> > > > + * to deal in the Software without restriction, including
> > > > without
> > > > limitation
> > > > + * the rights to use, copy, modify, merge, publish,
> > > > distribute,
> > > > sublicense,
> > > > + * and/or sell copies of the Software, and to permit persons
> > > > to
> > > > whom the
> > > > + * Software is furnished to do so, subject to the following
> > > > conditions:
> > > > + *
> > > > + * The above copyright notice and this permission notice
> > > > (including the next
> > > > + * paragraph) shall be included in all copies or substantial
> > > > portions of the
> > > > + * Software.
> > > > + *
> > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
> > > > KIND,
> > > > EXPRESS OR
> > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > > MERCHANTABILITY,
> > > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN
> > > > NO
> > > > EVENT SHALL
> > > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> > > > DAMAGES OR OTHER
> > > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> > > > OTHERWISE,
> > > > ARISING
> > > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
> > > > OR
> > > > OTHER DEALINGS
> > > > + * IN THE SOFTWARE.
> > > > + */
> > > > +
> > > > +#include <util/macros.h>
> > > > +#include <util/mesa-sha1.h>
> > > > +#include <main/mtypes.h>
> > > > +#include <compiler/glsl/glsl_parser_extras.h>
> > > > +#include <compiler/glsl/ir_uniform.h>
> > > > +#include <compiler/glsl/cache.h>
> > > > +#include <compiler/glsl/blob.h>
> > > > +
> > > > +#include "brw_state.h"
> > > > +#include "brw_wm.h"
> > > > +#include "brw_vs.h"
> > > > +#include "brw_context.h"
> > > > +
> > > > +static void
> > > > +gen_vs_sha1(struct brw_context *brw, struct gl_shader_program
> > > > *prog,
> > > > +            struct brw_vs_prog_key *vs_key, unsigned char
> > > > *vs_sha1)
> > > > +{
> > > > +   char sha1_buf[41];
> > > > +   unsigned char sha1[20];
> > > > +   char manifest[256];
> > > > +   int offset = 0;
> > > > +
> > > > +   offset += snprintf(manifest, sizeof(manifest), "program:
> > > > %s\n",
> > > > +                      _mesa_sha1_format(sha1_buf, prog-
> > > > >sha1));
> > > > +
> > > > +   _mesa_sha1_compute(vs_key, sizeof *vs_key, sha1);
> > > > +   offset += snprintf(manifest + offset, sizeof(manifest) -
> > > > offset,
> > > > +                      "vs_key: %s\n",
> > > > _mesa_sha1_format(sha1_buf,
> > > > sha1));
> > > > +
> > > > +   _mesa_sha1_compute(manifest, strlen(manifest), vs_sha1);
> > > > +}
> > > 
> > > The VS/TCS/TES/GS code is basically identical...you could avoid a
> > > lot
> > > of
> > > duplication by doing...
> > > 
> > > static void
> > > gen_shader_sha1(struct brw_context *brw, struct gl_shader_program
> > > *prog,
> > >                 unsigned stage, void *key, unsigned char
> > > *out_sha1)
> > > {
> > >    char sha1_buf[41];
> > >    unsigned char sha1[20];
> > >    char manifest[256];
> > >    int offset = 0;
> > > 
> > >    format_program_sha1(prog, manifest, sizeof(manifest),
> > > &offset);
> > > 
> > >    _mesa_sha1_compute(key, key_size(stage), sha1);
> > >    offset += snprintf(manifest + offset, sizeof(manifest) -
> > > offset,
> > >                       "%s_key: %s\n",
> > >                       _mesa_shader_stage_to_abbrev(stage),
> > >                       _mesa_sha1_format(sha1_buf, sha1));
> > > 
> > >    _mesa_sha1_compute(manifest, strlen(manifest), tcs_sha1)
> > > }
> > > 
> > > assuming you move the key initialization for TCS/TES/GS to the
> > > caller,
> > > which would make it more consistent with the VS anyway.  (Here,
> > > key_size
> > > is a helper function that returns sizeof(brw_vs_prog) etc.)
> > > 
> > > (Also assuming you're OK with using "VS_key" rather than
> > > "vs_key"...)
> > > 
> > > > 
> > > > 
> > > > +
> > > > +static void
> > > > +gen_wm_sha1(struct brw_context *brw, struct gl_shader_program
> > > > *prog,
> > > > +            struct brw_vs_prog_key *vs_key, struct
> > > > brw_wm_prog_key
> > > > *wm_key,
> > > > +            unsigned char *wm_sha1)
> > > > +{
> > > > +   char sha1_buf[41];
> > > > +   unsigned char sha1[20];
> > > > +   char manifest[256];
> > > > +   int offset = 0;
> > > > +
> > > > +   offset += snprintf(manifest, sizeof(manifest), "program:
> > > > %s\n",
> > > > +                      _mesa_sha1_format(sha1_buf, prog-
> > > > >sha1));
> > > > +
> > > > +   brw_wm_populate_key(brw, wm_key);
> > > > +   _mesa_sha1_compute(wm_key, sizeof *wm_key, sha1);
> > > > +   offset += snprintf(manifest + offset, sizeof(manifest) -
> > > > offset,
> > > > +                      "wm_key: %s\n",
> > > > _mesa_sha1_format(sha1_buf,
> > > > sha1));
> > > > +
> > > > +   _mesa_sha1_compute(manifest, strlen(manifest), wm_sha1);
> > > > +
> > > > +}
> > > 
> > > I don't know why this function is (eventually) monkeying around
> > > with
> > > the
> > > vue map coming out of the GS stage based on VS outputs
> > > written.  I
> > > can't
> > > imagine it works once you're caching TES/GS...
> > 
> > It's been a while since I worked on this but I believe this is
> > meant to
> > handle the effects of the following code in brw_state_upload()
> > 
> >    /* Update the VUE map for data exiting the GS stage of the
> > pipeline.
> >     * This comes from the last enabled shader stage.
> >     */
> >    GLbitfield64 old_slots = brw->vue_map_geom_out.slots_valid;
> >    bool old_separate = brw->vue_map_geom_out.separate;
> >    if (brw->geometry_program)
> >       brw->vue_map_geom_out = brw->gs.prog_data->base.vue_map;
> >    else if (brw->tess_eval_program)
> >       brw->vue_map_geom_out = brw->tes.prog_data->base.vue_map;
> >    else
> >       brw->vue_map_geom_out = brw->vs.prog_data->base.vue_map;
> 
> Sure, but it only ever looks at the VS...never the TES or GS.
> Even at the end of your series.  That can't work...

Looking at patch 54 more closely I think it's completely bogus. I think
it may have been an early hack around some problems that have since
been resolved after I re-wrote Carls early patches so that the cache
ordering is correct i.e checking the in memory cache before the on-disk 
cache.

It was also written before adding gs/tess. Anyway I've dropped that
patch and I'm running piglit now to make sure there are no regressions.


More information about the mesa-dev mailing list