On Wed, 2016-10-26 at 13:13 +0300, Tapani Pälli wrote: > 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@entry=0 > > > > x0, > > > > 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_o > > > > bjects > > > > /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?
ok, I see. The test says it *should* relink when in compat profile. In which case it should fall back to fixed-function. I'm not entirely sure what path things take from here but it seems something is not working correctly when creating the fallback shader. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 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=9771 > > > > > > 5 > > > > > > 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 > _______________________________________________ > 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
