[Mesa-dev] [PATCH 05/10] mesa: Additional SSO validation using program_interface_query data
Ian Romanick
idr at freedesktop.org
Tue May 24 18:28:00 UTC 2016
On 05/20/2016 06:03 PM, Timothy Arceri wrote:
> 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.
I'm not terribly worried about the performance here. We cache the
validation state, so we only pay the cost on the first draw call that
uses the pipeline in this particular state. This should still be better
than the previous validate_io function (removed in the next patch) which
loops over the shader IR. :)
>> + } 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