[Mesa-dev] [Mesa-stable] [PATCH v2 03/11] mesa: Strip arrayness from interface block names in some IO validation
Ian Romanick
idr at freedesktop.org
Thu Jun 23 22:07:14 UTC 2016
On 06/16/2016 12:12 PM, Ilia Mirkin wrote:
> On Thu, Jun 16, 2016 at 3:06 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Outputs from the vertex shader need to be able to match
>> per-vertex-arrayed inputs of later stages. Acomplish this by stripping
>> one level of arrayness from the names and types of outputs going to a
>> per-vertex-arrayed stage.
>>
>> v2: Add missing checks for TESS_EVAL->GEOMETRY. Noticed by Timothy
>> Arceri.
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96358
>> Cc: "12.0" <mesa-stable at lists.freedesktop.org>
>> Cc: Gregory Hainaut <gregory.hainaut at gmail.com>
>> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>> src/mesa/main/shader_query.cpp | 100 +++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 92 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
>> index 5956ce4..a6a0a2f 100644
>> --- a/src/mesa/main/shader_query.cpp
>> +++ b/src/mesa/main/shader_query.cpp
>> @@ -1385,13 +1385,26 @@ _mesa_get_program_resourceiv(struct gl_shader_program *shProg,
>>
>> static bool
>> validate_io(struct gl_shader_program *producer,
>> - struct gl_shader_program *consumer)
>> + struct gl_shader_program *consumer,
>> + gl_shader_stage producer_stage,
>> + gl_shader_stage consumer_stage)
>> {
>> if (producer == consumer)
>> return true;
>>
>> + const bool nonarray_stage_to_array_stage =
>> + (producer_stage == MESA_SHADER_VERTEX &&
>> + (consumer_stage == MESA_SHADER_GEOMETRY ||
>> + consumer_stage == MESA_SHADER_TESS_CTRL ||
>> + consumer_stage == MESA_SHADER_TESS_EVAL)) ||
>> + (producer_stage == MESA_SHADER_TESS_EVAL &&
>> + consumer_stage == MESA_SHADER_GEOMETRY);
>
> Since TCS -> GEOM is not possible, isn't the only way that array ->
> array can happen is TCS -> TES? IOW, couldn't this just be
>
> producer != TCS && (consumer == TCS || consumer == TES || consumer == GS)
>
> Your call whether you want to rewrite it. [No comment on the rest of
> the patch...]
I had though of another (slightly less) clever way to do this, but I
decided against it because it was less clear.
Anyway, I tried your suggestion. Here's the assembly 'diff
--side-by-side'. Mine is on the left, yours is on the right.
mov 0x4(%rax),%eax mov 0x4(%rax),%eax
cmp %rbp,%rbx cmp %rbp,%rbx
je 374c | je 3744
test %eax,%eax | movb $0x0,0x1f(%rsp)
jne 357b | cmp $0x1,%eax
lea -0x1(%rdx),%ecx | je 357f
movb $0x1,0x1f(%rsp) | sub $0x1,%edx
cmp $0x2,%ecx | cmp $0x2,%edx
jbe 358d | setbe 0x1f(%rsp)
cmp $0x2,%eax <
sete %cl <
cmp $0x3,%edx <
sete %al <
and %eax,%ecx <
mov %cl,0x1f(%rsp) <
I don't know that we'd be able to measure a performance difference.
Maybe having one less branch that may not predict well is good? *shrug*
> -ilia
>
>> +
>> bool valid = true;
>>
>> + void *name_buffer = NULL;
>> + size_t name_buffer_size = 0;
>> +
>> gl_shader_variable const **outputs =
>> (gl_shader_variable const **) calloc(producer->NumProgramResourceList,
>> sizeof(gl_shader_variable *));
>> @@ -1463,11 +1476,52 @@ validate_io(struct gl_shader_program *producer,
>> }
>> }
>> } else {
>> + char *consumer_name = consumer_var->name;
>> +
>> + if (nonarray_stage_to_array_stage &&
>> + consumer_var->interface_type != NULL &&
>> + consumer_var->interface_type->is_array() &&
>> + !is_gl_identifier(consumer_var->name)) {
>> + const size_t name_len = strlen(consumer_var->name);
>> +
>> + if (name_len >= name_buffer_size) {
>> + free(name_buffer);
>> +
>> + name_buffer_size = name_len + 1;
>> + name_buffer = malloc(name_buffer_size);
>> + if (name_buffer == NULL) {
>> + valid = false;
>> + goto out;
>> + }
>> + }
>> +
>> + consumer_name = (char *) name_buffer;
>> +
>> + char *s = strchr(consumer_var->name, '[');
>> + if (s == NULL) {
>> + valid = false;
>> + goto out;
>> + }
>> +
>> + char *t = strchr(s, ']');
>> + if (t == NULL) {
>> + valid = false;
>> + goto out;
>> + }
>> +
>> + assert(t[1] == '.' || t[1] == '[');
>> +
>> + const ptrdiff_t base_name_len = s - consumer_var->name;
>> +
>> + memcpy(consumer_name, consumer_var->name, base_name_len);
>> + strcpy(consumer_name + base_name_len, t + 1);
>> + }
>> +
>> 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) {
>> + strcmp(consumer_name, var->name) == 0) {
>> producer_var = var;
>> match_index = j;
>> break;
>> @@ -1529,25 +1583,53 @@ validate_io(struct gl_shader_program *producer,
>> * 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) {
>> + if (nonarray_stage_to_array_stage) {
>> + if (!consumer_var->type->is_array() ||
>> + consumer_var->type->fields.array != producer_var->type) {
>> + valid = false;
>> + goto out;
>> + }
>> +
>> + if (consumer_var->interface_type != NULL) {
>> + if (!consumer_var->interface_type->is_array() ||
>> + consumer_var->interface_type->fields.array != producer_var->interface_type) {
>> + valid = false;
>> + goto out;
>> + }
>> + } else if (producer_var->interface_type != NULL) {
>> + valid = false;
>> + goto out;
>> + }
>> + } else {
>> + if (producer_var->type != consumer_var->type) {
>> + valid = false;
>> + goto out;
>> + }
>> +
>> + if (producer_var->interface_type != consumer_var->interface_type) {
>> + valid = false;
>> + goto out;
>> + }
>> + }
>> +
>> + if (producer_var->interpolation != consumer_var->interpolation) {
>> valid = false;
>> goto out;
>> }
>>
>> - if (producer_var->outermost_struct_type != consumer_var->outermost_struct_type) {
>> + if (producer_var->precision != consumer_var->precision) {
>> valid = false;
>> goto out;
>> }
>>
>> - if (producer_var->interface_type != consumer_var->interface_type) {
>> + if (producer_var->outermost_struct_type != consumer_var->outermost_struct_type) {
>> valid = false;
>> goto out;
>> }
>> }
>>
>> out:
>> + free(name_buffer);
>> free(outputs);
>> return valid && num_outputs == 0;
>> }
>> @@ -1579,7 +1661,9 @@ _mesa_validate_pipeline_io(struct gl_pipeline_object *pipeline)
>> if (shProg[idx]->_LinkedShaders[idx]->Stage == MESA_SHADER_COMPUTE)
>> break;
>>
>> - if (!validate_io(shProg[prev], shProg[idx]))
>> + if (!validate_io(shProg[prev], shProg[idx],
>> + shProg[prev]->_LinkedShaders[prev]->Stage,
>> + shProg[idx]->_LinkedShaders[idx]->Stage))
>> return false;
>>
>> prev = idx;
>> --
>> 2.5.5
>>
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
>
More information about the mesa-dev
mailing list