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. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev