[Mesa-dev] [PATCH] st/mesa/i965: validate sampler type across the whole program
Tapani Pälli
tapani.palli at intel.com
Fri Apr 21 06:02:28 UTC 2017
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.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