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@entry=0x0, > predicate=predicate@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. > > > > > > > > > > This patch fixes regression > > > caused by 4542c7ed5fc6d8cb2495d322b4f06d802d7292cc. > > > > > > Signed-off-by: Tapani Pälli <tapani.pa...@intel.com> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97715 > > > Cc: "12.0 13.0" <mesa-sta...@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
