Timothy Arceri <t_arc...@yahoo.com.au> writes:
> Use common intrastage array validation for interface blocks. This change also
> allows us to support interface blocks that are arrays of arrays.
Please wrap this message to fit within 80 columns.
> ---
> src/glsl/link_interface_blocks.cpp | 76
> ++++++++++++++++++--------------------
> 1 file changed, 35 insertions(+), 41 deletions(-)
>
> diff --git a/src/glsl/link_interface_blocks.cpp
> b/src/glsl/link_interface_blocks.cpp
> index 0ce502d..9000a3c 100644
> --- a/src/glsl/link_interface_blocks.cpp
> +++ b/src/glsl/link_interface_blocks.cpp
> @@ -50,18 +50,20 @@ struct interface_block_definition
> * represents either the interface instance (for named interfaces), or a
> * member of the interface (for unnamed interfaces).
> */
> - explicit interface_block_definition(const ir_variable *var)
> - : type(var->get_interface_type()),
> - instance_name(NULL),
> - array_size(-1)
> + explicit interface_block_definition(ir_variable *var)
> + : var(var),
> + type(var->get_interface_type()),
> + instance_name(NULL)
> {
> if (var->is_interface_instance()) {
> instance_name = var->name;
> - if (var->type->is_array())
> - array_size = var->type->length;
> }
> explicitly_declared = (var->data.how_declared !=
> ir_var_declared_implicitly);
> }
> + /**
> + * Interface block ir_variable
> + */
> + ir_variable *var;
>
> /**
> * Interface block type
> @@ -74,12 +76,6 @@ struct interface_block_definition
> const char *instance_name;
>
> /**
> - * For an interface block array, the array size (or 0 if unsized).
> - * Otherwise -1.
> - */
> - int array_size;
> -
> - /**
> * True if this interface block was explicitly declared in the shader;
> * false if it was an implicitly declared built-in interface block.
> */
> @@ -95,7 +91,8 @@ struct interface_block_definition
> bool
> intrastage_match(interface_block_definition *a,
> const interface_block_definition *b,
^^^^^
I think the intent of this declaration is that b should not change (see below).
> - ir_variable_mode mode)
> + ir_variable_mode mode,
> + struct gl_shader_program *prog)
> {
> /* Types must match. */
> if (a->type != b->type) {
> @@ -120,18 +117,13 @@ intrastage_match(interface_block_definition *a,
> return false;
> }
>
> - /* Array vs. nonarray must be consistent, and sizes must be
> - * consistent, with the exception that unsized arrays match sized
> - * arrays.
> + /* If a block is an array then it must match across the shader.
> + * Unsized arrays are also processed and matched agaist sized arrays.
> */
> - if ((a->array_size == -1) != (b->array_size == -1))
> + if (b->var->type != a->var->type &&
> + (b->instance_name != NULL || a->instance_name != NULL) &&
> + !validate_intrastage_arrays(prog, b->var, a->var))
you pass a b's var member into validate_intrastage_arrays, where it may
be modified (due to your second patch in this series). Is that your intention?
It doesn't seem to affect any piglit tests.
> return false;
> - if (b->array_size != 0) {
> - if (a->array_size == 0)
> - a->array_size = b->array_size;
> - else if (a->array_size != b->array_size)
> - return false;
> - }
>
> return true;
> }
> @@ -150,12 +142,6 @@ interstage_match(const interface_block_definition
> *producer,
> const interface_block_definition *consumer,
> bool extra_array_level)
> {
> - /* Unsized arrays should not occur during interstage linking. They
> - * should have all been assigned a size by link_intrastage_shaders.
> - */
> - assert(consumer->array_size != 0);
> - assert(producer->array_size != 0);
> -
> /* Types must match. */
> if (consumer->type != producer->type) {
> /* Exception: if both the interface blocks are implicitly declared,
> @@ -165,20 +151,27 @@ interstage_match(const interface_block_definition
> *producer,
> if (consumer->explicitly_declared || producer->explicitly_declared)
> return false;
> }
> +
> + /* Ignore outermost array if geom shader */
> + const glsl_type *consumer_instance_type;
> if (extra_array_level) {
> - /* Consumer must be an array, and producer must not. */
> - if (consumer->array_size == -1)
> - return false;
> - if (producer->array_size != -1)
> - return false;
> + consumer_instance_type = consumer->var->type->fields.array;
> } else {
> - /* Array vs. nonarray must be consistent, and sizes must be consistent.
> - * Since unsized arrays have been ruled out, we can check this by just
> - * making sure the sizes are equal.
> - */
> - if (consumer->array_size != producer->array_size)
> + consumer_instance_type = consumer->var->type;
> + }
> +
> + /* If a block is an array then it must match across shaders.
> + * Since unsized arrays have been ruled out, we can check this by just
> + * making sure the types are equal.
> + */
> + if ((consumer->instance_name != NULL &&
> + consumer_instance_type->is_array()) ||
> + (producer->instance_name != NULL &&
> + producer->var->type->is_array())) {
> + if (consumer_instance_type != producer->var->type)
> return false;
> }
> +
> return true;
> }
>
> @@ -298,7 +291,8 @@ validate_intrastage_interface_blocks(struct
> gl_shader_program *prog,
> */
> definitions->store(def);
> } else if (!intrastage_match(prev_def, &def,
> - (ir_variable_mode) var->data.mode)) {
> + (ir_variable_mode) var->data.mode,
> + prog)) {
> linker_error(prog, "definitions of interface block `%s' do not"
> " match\n", iface_type->name);
> return;
> @@ -374,7 +368,7 @@ validate_interstage_uniform_blocks(struct
> gl_shader_program *prog,
> * uniform matchin rules (for uniforms, it is as though all
> * shaders are in the same shader stage).
> */
> - if (!intrastage_match(old_def, &new_def, ir_var_uniform)) {
> + if (!intrastage_match(old_def, &new_def, ir_var_uniform, prog)) {
> linker_error(prog, "definitions of interface block `%s' do
> not "
> "match\n", var->get_interface_type()->name);
> return;
> --
> 2.1.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
