On 04/19/2017 05:15 AM, Timothy Arceri wrote:
Hi Tapani,

You seem to have been one of the last people to touch this code. Do you think you could take a look at this patch?

OK sure.

The validation loop looks fine to me but I find it strange that we need to initialize SamplersValidated on each LinkShader hook separately, could we just do it once, perhaps _mesa_clear_shader_program_data() or such that gets called when starting to link?


Thanks,
Tim

On 13/04/17 12:01, Timothy Arceri wrote:
From: Timothy Arceri <timothy.arc...@collabora.com>

Currently we were only making sure types were the same within a
single stage. This looks to have regressed with 953a0af8e3f73.

https://bugs.freedesktop.org/show_bug.cgi?id=97524
---
 src/mesa/drivers/dri/i965/brw_link.cpp     |  1 +
 src/mesa/main/uniform_query.cpp            |  2 ++
src/mesa/main/uniforms.c | 26 +++++++++++++++++++++-----
 src/mesa/program/ir_to_mesa.cpp            |  1 +
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp |  1 +
 5 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp
index 7c10a40..7b3f8da 100644
--- a/src/mesa/drivers/dri/i965/brw_link.cpp
+++ b/src/mesa/drivers/dri/i965/brw_link.cpp
@@ -195,20 +195,21 @@ unify_interfaces(struct shader_info **infos)
 }

 extern "C" GLboolean
brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg)
 {
    struct brw_context *brw = brw_context(ctx);
    const struct brw_compiler *compiler = brw->screen->compiler;
    unsigned int stage;
    struct shader_info *infos[MESA_SHADER_STAGES] = { 0, };

+   shProg->SamplersValidated = GL_TRUE;
for (stage = 0; stage < ARRAY_SIZE(shProg->_LinkedShaders); stage++) {
       struct gl_linked_shader *shader = shProg->_LinkedShaders[stage];
       if (!shader)
          continue;

       struct gl_program *prog = shader->Program;
       prog->Parameters = _mesa_new_parameter_list();

       process_glsl_ir(brw, shProg, shader);

diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index 7aa035a..4d94e01 100644
--- a/src/mesa/main/uniform_query.cpp
+++ b/src/mesa/main/uniform_query.cpp
@@ -968,20 +968,22 @@ _mesa_uniform(GLint location, GLsizei count, const GLvoid *values,
       }
    }

    _mesa_propagate_uniforms_to_driver_storage(uni, offset, count);

/* If the uniform is a sampler, do the extra magic necessary to propagate
     * the changes through.
     */
    if (uni->type->is_sampler()) {
       bool flushed = false;
+      shProg->SamplersValidated = GL_TRUE;
+
       for (int i = 0; i < MESA_SHADER_STAGES; i++) {
      struct gl_linked_shader *const sh = shProg->_LinkedShaders[i];

/* If the shader stage doesn't use the sampler uniform, skip this. */
      if (!uni->opaque[i].active)
         continue;

          bool changed = false;
          for (int j = 0; j < count; j++) {
             unsigned unit = uni->opaque[i].index + offset + j;
diff --git a/src/mesa/main/uniforms.c b/src/mesa/main/uniforms.c
index 59ae4c5..8869b6e 100644
--- a/src/mesa/main/uniforms.c
+++ b/src/mesa/main/uniforms.c
@@ -60,42 +60,58 @@
  * So, scan the program->SamplerUnits[] and program->SamplerTargets[]
  * information to update the prog->TexturesUsed[] values.
  * Each value of TexturesUsed[unit] is one of zero, TEXTURE_1D_INDEX,
  * TEXTURE_2D_INDEX, TEXTURE_3D_INDEX, etc.
  * We'll use that info for state validation before rendering.
  */
 void
 _mesa_update_shader_textures_used(struct gl_shader_program *shProg,
                                   struct gl_program *prog)
 {
-   memset(prog->TexturesUsed, 0, sizeof(prog->TexturesUsed));
+   GLbitfield mask = prog->SamplersUsed;
+   gl_shader_stage prog_stage =
+      _mesa_program_enum_to_shader_stage(prog->Target);
+   struct gl_linked_shader *shader = shProg->_LinkedShaders[prog_stage];

-   shProg->SamplersValidated = GL_TRUE;
+   assert(shader);
+
+   memset(prog->TexturesUsed, 0, sizeof(prog->TexturesUsed));

-   GLbitfield mask = prog->SamplersUsed;
    while (mask) {
       const int s = u_bit_scan(&mask);
       GLuint unit = prog->SamplerUnits[s];
       GLuint tgt = prog->sh.SamplerTargets[s];
       assert(unit < ARRAY_SIZE(prog->TexturesUsed));
       assert(tgt < NUM_TEXTURE_TARGETS);

       /* The types of the samplers associated with a particular texture
* unit must be an exact match. Page 74 (page 89 of the PDF) of the
        * OpenGL 3.3 core spec says:
        *
        *     "It is not allowed to have variables of different sampler
* types pointing to the same texture image unit within a program
        *     object."
        */
-      if (prog->TexturesUsed[unit] & ~(1 << tgt))
-         shProg->SamplersValidated = GL_FALSE;
+      unsigned stages_mask = shProg->data->linked_stages;
+      while (stages_mask) {
+         const int stage = u_bit_scan(&stages_mask);
+
+ /* Skip validation if we are yet to update textures used in this
+          * stage.
+          */
+         if (prog_stage < stage)
+            break;
+
+ struct gl_program *glprog = shProg->_LinkedShaders[stage]->Program;
+         if (glprog->TexturesUsed[unit] & ~(1 << tgt))
+            shProg->SamplersValidated = GL_FALSE;
+      }

       prog->TexturesUsed[unit] |= (1 << tgt);
    }
 }

 /**
  * Connect a piece of driver storage with a part of a uniform
  *
* \param uni The uniform with which the storage will be associated
  * \param element_stride Byte-stride between array elements.
diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
index 6b33266..b12f5ce 100644
--- a/src/mesa/program/ir_to_mesa.cpp
+++ b/src/mesa/program/ir_to_mesa.cpp
@@ -3062,20 +3062,21 @@ _mesa_ir_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
                          options->EmitNoIndirectUniform)
          || progress;

      progress = do_vec_index_to_cond_assign(ir) || progress;
          progress = lower_vector_insert(ir, true) || progress;
       } while (progress);

       validate_ir_tree(ir);
    }

+   prog->SamplersValidated = GL_TRUE;
    for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
       struct gl_program *linked_prog;

       if (prog->_LinkedShaders[i] == NULL)
      continue;

linked_prog = get_mesa_program(ctx, prog, prog->_LinkedShaders[i]);

       if (linked_prog) {
          _mesa_copy_linked_program_data(prog, prog->_LinkedShaders[i]);
diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 96c08a6..fdc327c 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -6994,20 +6994,21 @@ st_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
             progress |= lower_if_to_cond_assign((gl_shader_stage)i, ir,
options->MaxIfDepth, if_threshold);
          } while (progress);
       }

       validate_ir_tree(ir);
    }

    build_program_resource_list(ctx, prog);

+   prog->SamplersValidated = GL_TRUE;
    for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
       struct gl_linked_shader *shader = prog->_LinkedShaders[i];
       if (shader == NULL)
          continue;

       enum pipe_shader_type ptarget =
          st_shader_stage_to_ptarget(shader->Stage);
       enum pipe_shader_ir preferred_ir = (enum pipe_shader_ir)
          pscreen->get_shader_param(pscreen, ptarget,
                                    PIPE_SHADER_CAP_PREFERRED_IR);

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to