[Mesa-dev] [PATCH 05/10] mesa: Additional SSO validation using program_interface_query data
Alejandro Piñeiro
apinheiro at igalia.com
Fri May 20 09:38:36 UTC 2016
On 20/05/16 09:26, 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_qualifier_vertex_smooth_fragment_flat
> dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_implicit_explicit_location_1
> dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_array_element_type
> dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_qualifier_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_qualifier_vertex_centroid_fragment_flat
> dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_array_length
> 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_explicit_location_type
> dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_qualifier_vertex_flat_fragment_centroid
> dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_explicit_location
> dEQP-GLES31.functional.separate_shader.validation.varying.mismatch_qualifier_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 you are going to add a spec quote to justify why built-ins are
skipped, probably it would be better to do it the first time you do it,
when filling outputs on the producer loop, just before.
> + 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;
> + }
> + }
> + } 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