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

Tapani Pälli tapani.palli at intel.com
Wed Oct 26 10:13:41 UTC 2016


Hi;

On 10/26/2016 11:27 AM, Tapani Pälli wrote:
>
>
> On 10/26/2016 11:21 AM, Timothy Arceri wrote:
>> On Wed, 2016-10-26 at 08:50 +0300, Tapani Pälli wrote:
>>>
>>> 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
>>
>> There is a comment above that line in the test.
>>
>> /* Issue #14 of the GL_ARB_separate_shader_objects spec says:
>> *
>>  *     "14. Should glLinkProgram work to re-link a shader created with
>>  *          glCreateShaderProgram?
>>  *
>>  *          RESOLVED: NO because the shader created by
>>  *          glCreateShaderProgram is detached and deleted as part of
>>  *          the glCreateShaderProgram sequence.  This means if you
>>  *          call glLinkProgram on a program returned from
>>  *          glCreateShaderProgram, you'll find the re-link fails
>>  *          because no shader object is attached.
>>  *
>>  *          An application is free to attach one or more new shader
>>  *          objects to the program and then relink would work.
>>  *
>>  *          This is fine because re-linking isn't necessary/expected."
>>  */
>>
>> Which means we shouldn't able to relink this shader. We shouldn't be
>> able to get as far along as we do when we get the error message.
>
> Ah right, so this should be detected somewhere within shaderapi, will
> take a look there. The reason I suspected we end here with SSO was that
> this fails just on the old gen models so something definitely goes
> different ways with those. But I'll look at shaderapi first.

Now I realized that I could bail in linker with following:
if (shProg->SeparateShader && shProg->NumShaders == 0)

BUT following commit states that having NumShaders == 0 is OK for 
compatibility profile:

76cfb472077dc83c892b4cddf79333341deaa7b5

Timothy, do you think I could still bailout with the condition above? I 
really do wonder the case mentioned in commit where someone links a 
shader without any stages, does this really make sense (what is the use 
case) or is this allowed just because spec does not happen to prohibit this?


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


More information about the mesa-stable mailing list