[Mesa-dev] [Mesa-stable] [PATCH] Revert "glsl: don't reference shader prog data during cache fallback"

Juan A. Suarez Romero jasuarez at igalia.com
Thu Jun 1 07:05:13 UTC 2017


On Thu, 2017-06-01 at 10:09 +1000, Timothy Arceri wrote:
> On 01/06/17 07:30, Juan A. Suarez Romero wrote:
> > On Tue, 2017-05-16 at 20:40 +0200, Nicolai Hähnle wrote:
> > > From: Nicolai Hähnle <nicolai.haehnle at amd.com>
> > > 
> > > This reverts commit 0e9991f957e296f46cfff40a94ffba0adf2a58e1.
> > > 
> > > At least when we fallback because of TGSI, the gl_program objects are
> > > re-allocated, and we don't in fact already have a reference to the
> > > gl_shader_program_data.
> > > 
> > 
> > This patch didn't land in master yet. Any news on this?
> > 
> > Also, commit 80e64334 (st/mesa: don't mark the program as in
> > cache_fallback when there is cache miss) fixes the patch we are
> > reverting. Probably we will need to revert too.
> > 
> > 
> > 	J.A.
> 
> Hi,
> 
> First of all sorry for the confusion. Please ignore this patch, 80e64334 
> is the one we would like to land in stable.
> 
> Please let me know if there are any issue applying that one to stable.
> 

Thank you! I already queued that patch and found no issues so far.


	J.A.

> Thanks,
> Tim
> 
> > 
> > 
> > 
> > > This fixes a crash that can be reproduced as follows:
> > > 
> > > 1. Run any GL program with MESA_GLSL=cache_info.
> > > 2. Note the SHA of some tgsi_tokens.
> > > 3. Remove the corresponding file from ~/.cache/mesa and re-run.
> > > 
> > > Cc: 17.1 <mesa-stable at lists.freedesktop.org>
> > > ---
> > >   src/compiler/glsl/linker.cpp | 3 +--
> > >   src/mesa/main/shaderobj.c    | 3 +--
> > >   2 files changed, 2 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> > > index 2e7dd2b..4776d09 100644
> > > --- a/src/compiler/glsl/linker.cpp
> > > +++ b/src/compiler/glsl/linker.cpp
> > > @@ -2243,22 +2243,21 @@ link_intrastage_shaders(void *mem_ctx,
> > >      struct gl_program *gl_prog =
> > >         ctx->Driver.NewProgram(ctx,
> > >                                _mesa_shader_stage_to_program(shader_list[0]->Stage),
> > >                                prog->Name, false);
> > >      if (!gl_prog) {
> > >         prog->data->LinkStatus = linking_failure;
> > >         _mesa_delete_linked_shader(ctx, linked);
> > >         return NULL;
> > >      }
> > >   
> > > -   if (!prog->data->cache_fallback)
> > > -      _mesa_reference_shader_program_data(ctx, &gl_prog->sh.data, prog->data);
> > > +   _mesa_reference_shader_program_data(ctx, &gl_prog->sh.data, prog->data);
> > >   
> > >      /* Don't use _mesa_reference_program() just take ownership */
> > >      linked->Program = gl_prog;
> > >   
> > >      linked->ir = new(linked) exec_list;
> > >      clone_ir_list(mem_ctx, linked->ir, main->ir);
> > >   
> > >      link_fs_inout_layout_qualifiers(prog, linked, shader_list, num_shaders);
> > >      link_tcs_out_layout_qualifiers(prog, gl_prog, shader_list, num_shaders);
> > >      link_tes_in_layout_qualifiers(prog, gl_prog, shader_list, num_shaders);
> > > diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
> > > index 8a5fa5e..f6f7bf5 100644
> > > --- a/src/mesa/main/shaderobj.c
> > > +++ b/src/mesa/main/shaderobj.c
> > > @@ -427,22 +427,21 @@ _mesa_free_shader_program_data(struct gl_context *ctx,
> > >   
> > >   
> > >   /**
> > >    * Free/delete a shader program object.
> > >    */
> > >   void
> > >   _mesa_delete_shader_program(struct gl_context *ctx,
> > >                               struct gl_shader_program *shProg)
> > >   {
> > >      _mesa_free_shader_program_data(ctx, shProg);
> > > -   if (!shProg->data->cache_fallback)
> > > -      _mesa_reference_shader_program_data(ctx, &shProg->data, NULL);
> > > +   _mesa_reference_shader_program_data(ctx, &shProg->data, NULL);
> > >      ralloc_free(shProg);
> > >   }
> > >   
> > >   
> > >   /**
> > >    * Lookup a GLSL program object.
> > >    */
> > >   struct gl_shader_program *
> > >   _mesa_lookup_shader_program(struct gl_context *ctx, GLuint name)
> > >   {
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > 
> 
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable


More information about the mesa-dev mailing list