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=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.

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

Reply via email to