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

[Nicolai Hähnle]

On 17.05.2017 01:49, Timothy Arceri wrote:
> This might work for gallium based drivers but I'm pretty sure this will
> cause problems for the i965 fallback path.

What do those problems look like? And any ideas on how to address the 
issue then? There are a number of places that assume prog->sh.data is 
non-NULL.

Cheers,
Nicolai


>
> On 17/05/17 04:40, 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 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


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.