[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