[Mesa-stable] [Mesa-dev] [PATCH] i965: do not release GLSL IR for SSO programs

[Tapani Pälli]

On 10/26/2016 08:15 AM, Timothy Arceri wrote:
> On Tue, 2016-10-25 at 09:39 +0300, Tapani Pälli wrote:
>> SSO shader programs can be later modified by attaching/detaching
>> shaders and relinked, this requires IR.
>
> Doesn't relinking recreate the IR? We can relink exiting shaders into
> new programs. The IR is cloned from gl_shader (the compiled IR) before
> this happens.
>
> Where exactly are things falling over for SSO?

I went this way as I haven't seen this happening elsewhere but when 
relinking program created by glCreateShaderProgram. TBH I'm not sure if 
this could happen with regular programs, I would assume we had already 
bugs if it would be so (?)

In practice things go bad in brw_link_shader where process_glsl_ir() 
gets called, like this:

--- 8< ---
#0  do_expression_flattening (instructions=instructions at entry=0x0, 
predicate=predicate at entry=0x7ffff0f808a0 
<mat_op_to_vec_predicate(ir_instruction*)>) at 
glsl/ir_expression_flattening.cpp:60
#1  0x00007ffff0f816b9 in do_mat_op_to_vec (instructions=0x0) at 
glsl/lower_mat_op_to_vec.cpp:96
#2  0x00007ffff10385bd in process_glsl_ir (shader_prog=0x9b74d8, 
shader=0x9b2688, brw=0x7d2478) at brw_link.cpp:108
#3  brw_link_shader (ctx=0x7d2478, shProg=0x9b74d8) at brw_link.cpp:234
#4  0x00007ffff0ec1f31 in _mesa_glsl_link_shader (ctx=0x7d2478, 
prog=0x9b74d8) at program/ir_to_mesa.cpp:3067
#5  0x00007ffff0ddc99b in _mesa_link_program (ctx=0x7d2478, 
shProg=0x9b74d8) at main/shaderapi.c:1098
#6  0x00007ffff7ac8d15 in stub_glLinkProgram (program=2) at 
/home/tpalli/source/fdo/piglit/tests/util/piglit-dispatch-gen.c:33005
#7  0x00000000004019ab in 
relink_program_created_by_glCreateShaderProgram () at 
/home/tpalli/source/fdo/piglit/tests/spec/arb_separate_shader_objects/api-errors.c:78


>>  This patch fixes regression
>> caused by 4542c7ed5fc6d8cb2495d322b4f06d802d7292cc.
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97715
>> Cc: "12.0 13.0" <mesa-stable at lists.freedesktop.org>
>> ---
>>  src/mesa/drivers/dri/i965/brw_link.cpp | 17 ++++++++++-------
>>  1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp
>> b/src/mesa/drivers/dri/i965/brw_link.cpp
>> index 5ea9773..ffb66a9 100644
>> --- a/src/mesa/drivers/dri/i965/brw_link.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_link.cpp
>> @@ -290,14 +290,17 @@ brw_link_shader(struct gl_context *ctx, struct
>> gl_shader_program *shProg)
>>
>>     build_program_resource_list(ctx, shProg);
>>
>> -   for (stage = 0; stage < ARRAY_SIZE(shProg->_LinkedShaders);
>> stage++) {
>> -      struct gl_linked_shader *shader = shProg-
>>> _LinkedShaders[stage];
>> -      if (!shader)
>> -         continue;
>> +   /* We can't free IR for SSO programs since those may need
>> relinking. */
>> +   if (!shProg->SeparateShader) {
>> +      for (stage = 0; stage < ARRAY_SIZE(shProg->_LinkedShaders);
>> stage++) {
>> +         struct gl_linked_shader *shader = shProg-
>>> _LinkedShaders[stage];
>> +         if (!shader)
>> +            continue;
>>
>> -      /* The GLSL IR won't be needed anymore. */
>> -      ralloc_free(shader->ir);
>> -      shader->ir = NULL;
>> +         /* The GLSL IR won't be needed anymore. */
>> +         ralloc_free(shader->ir);
>> +         shader->ir = NULL;
>> +      }
>>     }
>>
>>     return true;