[Mesa-dev] [PATCH] st/mesa/i965: validate sampler type across the whole program

Timothy Arceri tarceri at itsqueeze.com
Wed Apr 19 02:15:58 UTC 2017


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?

Thanks,
Tim

On 13/04/17 12:01, Timothy Arceri wrote:
> From: Timothy Arceri <timothy.arceri at 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);
>


More information about the mesa-dev mailing list