[Mesa-dev] [PATCH 05/10] mesa: Additional SSO validation using program_interface_query data

Timothy Arceri timothy.arceri at collabora.com
Sat May 21 01:03:56 UTC 2016


On Fri, 2016-05-20 at 00:26 -0700, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> Fixes the following dEQP tests on SKL:
> 
> dEQP-
> GLES31.functional.separate_shader.validation.varying.mismatch_qualifi
> er_vertex_smooth_fragment_flat
> dEQP-
> GLES31.functional.separate_shader.validation.varying.mismatch_implici
> t_explicit_location_1
> dEQP-
> GLES31.functional.separate_shader.validation.varying.mismatch_array_e
> lement_type
> dEQP-
> GLES31.functional.separate_shader.validation.varying.mismatch_qualifi
> er_vertex_flat_fragment_none
> dEQP-
> GLES31.functional.separate_shader.validation.varying.mismatch_struct_
> member_order
> dEQP-
> GLES31.functional.separate_shader.validation.varying.mismatch_struct_
> member_type
> dEQP-
> GLES31.functional.separate_shader.validation.varying.mismatch_qualifi
> er_vertex_centroid_fragment_flat
> dEQP-
> GLES31.functional.separate_shader.validation.varying.mismatch_array_l
> ength
> dEQP-
> GLES31.functional.separate_shader.validation.varying.mismatch_type
> dEQP-
> GLES31.functional.separate_shader.validation.varying.mismatch_struct_
> member_precision
> dEQP-
> GLES31.functional.separate_shader.validation.varying.mismatch_explici
> t_location_type
> dEQP-
> GLES31.functional.separate_shader.validation.varying.mismatch_qualifi
> er_vertex_flat_fragment_centroid
> dEQP-
> GLES31.functional.separate_shader.validation.varying.mismatch_explici
> t_location
> dEQP-
> GLES31.functional.separate_shader.validation.varying.mismatch_qualifi
> er_vertex_flat_fragment_smooth
> dEQP-
> GLES31.functional.separate_shader.validation.varying.mismatch_struct_
> member_name
> 
> It regresses one test:
> 
> dEQP-
> GLES31.functional.separate_shader.validation.varying.match_different_
> struct_names
> 
> Hoever, this test is based on language in the OpenGL ES 3.1 spec that
> I
> believe is incorrect.  I have already submitted a spec bug:
> 
> https://www.khronos.org/bugzilla/show_bug.cgi?id=1500
> 
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
> 
> I'm experimenting with different formatting of spec quotations.  This
> is
> something new-ish based on the list discussion with Ken and Matt
> earlier
> this week.
> 
>  src/mesa/main/shader_query.cpp | 172
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 172 insertions(+)
> 
> diff --git a/src/mesa/main/shader_query.cpp
> b/src/mesa/main/shader_query.cpp
> index a120cb4..5fa611f 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -1470,6 +1470,175 @@ validate_io(const struct gl_shader *producer,
>     return inputs == outputs;
>  }
>  
> +static bool
> +validate_io(struct gl_shader_program *producer,
> +            struct gl_shader_program *consumer)
> +{
> +   if (producer == consumer)
> +      return true;
> +
> +   bool valid = true;
> +
> +   gl_shader_variable const **outputs =
> +      (gl_shader_variable const **) calloc(producer-
> >NumProgramResourceList,
> +                                           sizeof(gl_shader_variable
> *));
> +   if (outputs == NULL)
> +      return false;
> +
> +   /* Section 7.4.1 (Shader Interface Matching) of the OpenGL ES 3.1
> spec
> +    * says:
> +    *
> +    *    At an interface between program objects, the set of inputs
> and
> +    *    outputs are considered to match exactly if and only if:
> +    *
> +    *    - Every declared input variable has a matching output, as
> described
> +    *      above.
> +    *    - There are no user-defined output variables declared
> without a
> +    *      matching input variable declaration.
> +    *
> +    * Every input has an output, and every output has an
> input.  Scan the list
> +    * of producer resources once, and generate the list of
> outputs.  As inputs
> +    * and outputs are matched, remove the matched outputs from the
> set.  At
> +    * the end, the set must be empty.  If the set is not empty, then
> there is
> +    * some output that did not have an input.
> +    */
> +   unsigned num_outputs = 0;
> +   for (unsigned i = 0; i < producer->NumProgramResourceList; i++) {
> +      struct gl_program_resource *res = &producer-
> >ProgramResourceList[i];
> +
> +      if (res->Type != GL_PROGRAM_OUTPUT)
> +         continue;
> +
> +      gl_shader_variable const *const var = RESOURCE_VAR(res);
> +
> +      if (is_gl_identifier(var->name))
> +         continue;
> +
> +      outputs[num_outputs++] = var;
> +   }
> +
> +   unsigned match_index = 0;
> +   for (unsigned i = 0; i < consumer->NumProgramResourceList; i++) {
> +      struct gl_program_resource *res = &consumer-
> >ProgramResourceList[i];
> +
> +      if (res->Type != GL_PROGRAM_INPUT)
> +         continue;
> +
> +      gl_shader_variable const *const consumer_var =
> RESOURCE_VAR(res);
> +      gl_shader_variable const *producer_var = NULL;
> +
> +      /* Section 7.4.1 (Shader Interface Matching) of the OpenGL ES
> 3.1 spec
> +       * says:
> +       *
> +       *    Built-in inputs or outputs do not affect interface
> matching.
> +       */
> +      if (is_gl_identifier(consumer_var->name))
> +         continue;
> +
> +      /* Inputs with explicit locations match other outputs with
> explicit
> +       * locations by location instead of by name.
> +       */
> +      if (consumer_var->explicit_location) {
> +         for (unsigned j = 0; j < num_outputs; j++) {
> +            const gl_shader_variable *const var = outputs[j];
> +
> +            if (var->explicit_location &&
> +                consumer_var->location == var->location) {
> +               producer_var = var;
> +               match_index = j;
> +               break;
> +            }
> +         }

Considering this is called a draw time would it be better to presort
explicit locations in the outputs array like we do
in cross_validate_outputs_to_inputs() during linking? Rather
than continually looping over the array.

> +      } else {
> +         for (unsigned j = 0; j < num_outputs; j++) {
> +            const gl_shader_variable *const var = outputs[j];
> +
> +            if (!var->explicit_location &&
> +                strcmp(consumer_var->name, var->name) == 0) {
> +               producer_var = var;
> +               match_index = j;
> +               break;
> +            }
> +         }
> +      }
> +
> +      /* Section 7.4.1 (Shader Interface Matching) of the OpenGL ES
> 3.1 spec
> +       * says:
> +       *
> +       *    - An output variable is considered to match an input
> variable in
> +       *      the subsequent shader if:
> +       *
> +       *      - the two variables match in name, type, and
> qualification; or
> +       *
> +       *      - the two variables are declared with the same
> location
> +       *        qualifier and match in type and qualification.
> +       */
> +      if (producer_var == NULL) {
> +         valid = false;
> +         goto out;
> +      }
> +
> +      /* An output cannot match more than one input, so remove the
> output from
> +       * the set of possible outputs.
> +       */
> +      outputs[match_index] = NULL;
> +      num_outputs--;
> +      if (match_index < num_outputs)
> +         outputs[match_index] = outputs[num_outputs];
> +
> +      /* Section 9.2.2 (Separable Programs) of the GLSL ES spec
> says:
> +       *
> +       *    Qualifier Class|  Qualifier  |in/out
> +       *    ---------------+-------------+------
> +       *    Storage        |     in      |
> +       *                   |     out     |  N/A
> +       *                   |   uniform   |
> +       *    ---------------+-------------+------
> +       *    Auxiliary      |   centroid  |   No
> +       *    ---------------+-------------+------
> +       *                   |   location  |  Yes
> +       *                   | Block layout|  N/A
> +       *                   |   binding   |  N/A
> +       *                   |   offset    |  N/A
> +       *                   |   format    |  N/A
> +       *    ---------------+-------------+------
> +       *    Interpolation  |   smooth    |
> +       *                   |    flat     |  Yes
> +       *    ---------------+-------------+------
> +       *                   |    lowp     |
> +       *    Precision      |   mediump   |  Yes
> +       *                   |    highp    |
> +       *    ---------------+-------------+------
> +       *    Variance       |  invariant  |   No
> +       *    ---------------+-------------+------
> +       *    Memory         |     all     |  N/A
> +       *
> +       * Note that location mismatches are detected by the loops
> above that
> +       * find the producer variable that goes with the consumer
> variable.
> +       */
> +      if (producer_var->type != consumer_var->type ||
> +          producer_var->interpolation != consumer_var->interpolation 
> ||
> +          producer_var->precision != consumer_var->precision) {
> +         valid = false;
> +         goto out;
> +      }
> +
> +      if (producer_var->outermost_struct_type != consumer_var-
> >outermost_struct_type) {
> +         valid = false;
> +         goto out;
> +      }
> +
> +      if (producer_var->interface_type != consumer_var-
> >interface_type) {
> +         valid = false;
> +         goto out;
> +      }
> +   }
> +
> + out:
> +   free(outputs);
> +   return valid && num_outputs == 0;
> +}
> +
>  /**
>   * Validate inputs against outputs in a program pipeline.
>   */
> @@ -1501,6 +1670,9 @@ _mesa_validate_pipeline_io(struct
> gl_pipeline_object *pipeline)
>              if (!validate_io(shProg[prev]->_LinkedShaders[prev],
>                               shProg[idx]->_LinkedShaders[idx]))
>                 return false;
> +
> +            if (!validate_io(shProg[prev], shProg[idx]))
> +               return false;
>           }
>  
>           prev = idx;


More information about the mesa-dev mailing list