On Wed, Oct 26, 2016 at 8:08 PM, Timothy Arceri <timothy.arc...@collabora.com> wrote: > On Wed, 2016-10-26 at 22:51 +1100, Timothy Arceri wrote: >> 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. > > Actually we still return early it just shouldn't throw an error. > >> 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. > > I've taken a closer look. We don't do anything about creating fallback > shaders in GLSL IR so we shouldn't be hitting the code where the error > happens. > > I believe the problem is in glDetachShader() we remove the gl_shader > but we leave gl_linked_shader in place.
AFAIK that's a valid sequence - glAttachShader(); glLinkProgram(); glDetachShader(); glDeleteShader(); - the program should still be usable. -ilia _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev