[Mesa-dev] [PATCH 7/7] mesa: validate precision of varyings during ValidateProgramPipeline
Iago Toral
itoral at igalia.com
Tue Nov 10 03:44:00 PST 2015
On Tue, 2015-11-10 at 13:34 +0200, Tapani Pälli wrote:
>
> On 11/10/2015 01:15 PM, Iago Toral wrote:
> > On Tue, 2015-11-10 at 12:10 +0100, Iago Toral wrote:
> >> On Thu, 2015-11-05 at 13:33 +0200, Tapani Pälli wrote:
> >>> Fixes following failing ES3.1 CTS tests:
> >>>
> >>> ES31-CTS.sepshaderobjs.InterfacePrecisionMatchingFloat
> >>> ES31-CTS.sepshaderobjs.InterfacePrecisionMatchingInt
> >>> ES31-CTS.sepshaderobjs.InterfacePrecisionMatchingUInt
> >>>
> >>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> >>> ---
> >>> src/mesa/main/pipelineobj.c | 15 ++++++++++++
> >>> src/mesa/main/shader_query.cpp | 55 ++++++++++++++++++++++++++++++++++++++++++
> >>> src/mesa/main/shaderobj.h | 3 +++
> >>> 3 files changed, 73 insertions(+)
> >>>
> >>> diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
> >>> index 699a2ae..90dff13 100644
> >>> --- a/src/mesa/main/pipelineobj.c
> >>> +++ b/src/mesa/main/pipelineobj.c
> >>> @@ -907,6 +907,21 @@ _mesa_ValidateProgramPipeline(GLuint pipeline)
> >>>
> >>> _mesa_validate_program_pipeline(ctx, pipe,
> >>> (ctx->_Shader->Name == pipe->Name));
> >>> +
> >>> + /* Validate inputs against outputs, this cannot be done during linking
> >>> + * since programs have been linked separately from each other.
> >>> + *
> >>> + * From OpenGL 4.5 Core spec:
> >>> + * "Separable program objects may have validation failures that cannot be
> >>> + * detected without the complete program pipeline. Mismatched interfaces,
> >>> + * improper usage of program objects together, and the same
> >>> + * state-dependent failures can result in validation errors for such
> >>> + * program objects."
> >>> + *
> >>> + * OpenGL ES 3.1 specification has the same text.
> >>> + */
> >>> + if (!_mesa_validate_pipeline_io(pipe))
> >>> + pipe->Validated = GL_FALSE;
> >>> }
> >>>
> >>> void GLAPIENTRY
> >>> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> >>> index 5cb877b..595bdea 100644
> >>> --- a/src/mesa/main/shader_query.cpp
> >>> +++ b/src/mesa/main/shader_query.cpp
> >>> @@ -1359,3 +1359,58 @@ _mesa_get_program_resourceiv(struct gl_shader_program *shProg,
> >>> if (length)
> >>> *length = amount;
> >>> }
> >>> +
> >>> +static bool
> >>> +validate_io(const struct gl_shader *input_stage,
> >>> + const struct gl_shader *output_stage)
> >>> +{
> >>> + assert(input_stage && output_stage);
> >>
> >> Maybe add the relevant spec quote for this:
> >>
> >> "When both shaders are in separate programs, mismatched precision
> >> qualifiers will result in a program interface mismatch that will
> >> result in program pipeline validation failures"
> >
> > Sorry, this was cut, the complete text reads:
> >
> > "When both shaders are in separate programs, mismatched precision
> > qualifiers will result in a program interface mismatch that will result
> > in program pipeline validation failures, as described in section 7.4.1
> > (“Shader Interface Matching”) of the OpenGL ES 3.1 Specification."
> >
> > This text is from chapter "4.7.3. Precision Qualifiers" of the OpenGL ES
> > 3.1 spec.
>
> OK, I'll add spec quote.
>
> >>> + /* For each output in a, find input in b and do any required checks. */
> >>
> >> s/a/input_stage
> >> s/b/output_stage
> >>
> >>> + foreach_in_list(ir_instruction, out, input_stage->ir) {
> >>> + ir_variable *out_var = out->as_variable();
> >>> + if (!out_var || out_var->data.mode != ir_var_shader_out)
> >>> + continue;
> >>> +
> >>> + foreach_in_list(ir_instruction, in, output_stage->ir) {
> >>> + ir_variable *in_var = in->as_variable();
> >>> + if (!in_var || in_var->data.mode != ir_var_shader_in)
> >>> + continue;
> >>> +
> >>> + if (strcmp(in_var->name, out_var->name) == 0) {
> >>> + if (in_var->data.precision != out_var->data.precision)
> >>> + return false;
> >>> + }
> >>> + }
> >>
> >> This is O(n²) but we can easily make it O(n) by moving the inner loop
> >> outside and putting the input variables we find there in a hash table.
> >> Then the loop over input_stage just needs to do hash table lookups
> >> instead of looping for each output variable it has. What do you think?
>
> I'm not sure if it's worth the trouble but I can take a try. We would
> need to also destroy hash which will take some time.
Destroying the table should be faster than iterating over the list of
instructions for each output var, but it was only a suggestion, so feel
free to ignore it if you're not convinced it is worth it. We can always
make that change later if we want, so feel free to add my Rb as it is.
Iago
> I guess in long run this should be implemented by iterating program
> resource list and maybe those should be also split to resource pools
> instead of one single list. I chose single list originally because
> typically that list is quite small (we are talking about ~10 to 20, not
> hundreds) and there are api calls that iterate through all resources at
> once. However I would like to do that change only when quest for GLES
> 3.1 is over.
>
>
> >> Iago
> >>
> >>> + }
> >>> + return true;
> >>> +}
> >>> +
> >>> +/**
> >>> + * Validate inputs against outputs in a program pipeline.
> >>> + */
> >>> +extern "C" bool
> >>> +_mesa_validate_pipeline_io(struct gl_pipeline_object *pipeline)
> >>> +{
> >>> + struct gl_shader_program **shProg =
> >>> + (struct gl_shader_program **) pipeline->CurrentProgram;
> >>> +
> >>> + /* Find first active stage in pipeline. */
> >>> + unsigned idx, prev = 0;
> >>> + for (idx = 0; idx < ARRAY_SIZE(pipeline->CurrentProgram); idx++) {
> >>> + if (shProg[idx]) {
> >>> + prev = idx;
> >>> + break;
> >>> + }
> >>> + }
> >>> +
> >>> + for (idx = prev + 1; idx < ARRAY_SIZE(pipeline->CurrentProgram); idx++) {
> >>> + if (shProg[idx]) {
> >>> + if (!validate_io(shProg[prev]->_LinkedShaders[prev],
> >>> + shProg[idx]->_LinkedShaders[idx]))
> >>> + return false;
> >>> + prev = idx;
> >>> + }
> >>> + }
> >>> + return true;
> >>> +}
> >>> diff --git a/src/mesa/main/shaderobj.h b/src/mesa/main/shaderobj.h
> >>> index 796de47..be80752 100644
> >>> --- a/src/mesa/main/shaderobj.h
> >>> +++ b/src/mesa/main/shaderobj.h
> >>> @@ -234,6 +234,9 @@ _mesa_shader_stage_to_subroutine_uniform(gl_shader_stage stage)
> >>> }
> >>> }
> >>>
> >>> +extern bool
> >>> +_mesa_validate_pipeline_io(struct gl_pipeline_object *);
> >>> +
> >>> #ifdef __cplusplus
> >>> }
> >>> #endif
> >>
> >
> >
>
More information about the mesa-dev
mailing list