Re: [Mesa-dev] [PATCH v2 03/11] mesa: Strip arrayness from interface block names in some IO validation
On Thursday, June 16, 2016 12:06:56 PM PDT Ian Romanick wrote: > From: Ian Romanick > > 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 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96358 > Cc: "12.0" > Cc: Gregory Hainaut > Cc: Ilia Mirkin This is an annoying amount of code, but it looks OK to me and I don't have any ideas of how to do it better. Reviewed-by: Kenneth Graunke signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 03/11] mesa: Strip arrayness from interface block names in some IO validation
ping I'll have a response to Ilia's feedback shortly. On 06/16/2016 12:06 PM, Ian Romanick wrote: > From: Ian Romanick > > 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 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96358 > Cc: "12.0" > Cc: Gregory Hainaut > Cc: Ilia Mirkin > --- > 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); > + > 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; > +
Re: [Mesa-dev] [PATCH v2 03/11] mesa: Strip arrayness from interface block names in some IO validation
On Thu, Jun 16, 2016 at 3:06 PM, Ian Romanick wrote: > From: Ian Romanick > > 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 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96358 > Cc: "12.0" > Cc: Gregory Hainaut > Cc: Ilia Mirkin > --- > 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...] -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_typ
[Mesa-dev] [PATCH v2 03/11] mesa: Strip arrayness from interface block names in some IO validation
From: Ian Romanick 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 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96358 Cc: "12.0" Cc: Gregory Hainaut Cc: Ilia Mirkin --- 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); + 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 != consume