[Mesa-dev] [PATCH 18/19] glsl: allow AoA to be sized by initializer or constructor
Timothy Arceri
t_arceri at yahoo.com.au
Sat Jun 20 05:59:29 PDT 2015
On Sat, 2015-06-20 at 22:33 +1000, Timothy Arceri wrote:
> From Section 4.1.9 of the GLSL ES 3.10 spec:
>
> "Arrays are sized either at compile-time or at run-time.
> To size an array at compile-time, either the size
> must be specified within the brackets as above or
> must be inferred from the type of the initializer."
> ---
> src/glsl/ast.h | 21 +++---------
> src/glsl/ast_array_index.cpp | 7 ++--
> src/glsl/ast_function.cpp | 33 +++++++++++++++++-
> src/glsl/ast_to_hir.cpp | 80 ++++++++++++++++++++++++++++++++++----------
> src/glsl/glsl_parser.yy | 11 +++---
> 5 files changed, 107 insertions(+), 45 deletions(-)
>
> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> index 3f67907..0f9dbf9 100644
> --- a/src/glsl/ast.h
> +++ b/src/glsl/ast.h
> @@ -181,6 +181,7 @@ enum ast_operators {
> ast_post_dec,
> ast_field_selection,
> ast_array_index,
> + ast_unsized_array_dim,
>
> ast_function_call,
>
> @@ -308,16 +309,7 @@ private:
>
> class ast_array_specifier : public ast_node {
> public:
> - /** Unsized array specifier ([]) */
> - explicit ast_array_specifier(const struct YYLTYPE &locp)
> - : is_unsized_array(true)
> - {
> - set_location(locp);
> - }
> -
> - /** Sized array specifier ([dim]) */
> ast_array_specifier(const struct YYLTYPE &locp, ast_expression *dim)
> - : is_unsized_array(false)
> {
> set_location(locp);
> array_dimensions.push_tail(&dim->link);
> @@ -330,19 +322,14 @@ public:
>
> bool is_single_dimension()
> {
> - return (this->is_unsized_array && this->array_dimensions.is_empty()) ||
> - (!this->is_unsized_array &&
> - this->array_dimensions.tail_pred->prev != NULL &&
> - this->array_dimensions.tail_pred->prev->is_head_sentinel());
> + return this->array_dimensions.tail_pred->prev != NULL &&
> + this->array_dimensions.tail_pred->prev->is_head_sentinel();
> }
>
> virtual void print(void) const;
>
> - /* If true, this means that the array has an unsized outermost dimension. */
> - bool is_unsized_array;
> -
> /* This list contains objects of type ast_node containing the
> - * sized dimensions only, in outermost-to-innermost order.
> + * array dimensions in outermost-to-innermost order.
> */
> exec_list array_dimensions;
> };
> diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
> index 752d86f..89c08d8 100644
> --- a/src/glsl/ast_array_index.cpp
> +++ b/src/glsl/ast_array_index.cpp
> @@ -28,13 +28,10 @@
> void
> ast_array_specifier::print(void) const
> {
> - if (this->is_unsized_array) {
> - printf("[ ] ");
> - }
> -
> foreach_list_typed (ast_node, array_dimension, link, &this->array_dimensions) {
> printf("[ ");
> - array_dimension->print();
> + if (((ast_expression*)array_dimension)->oper != ast_unsized_array_dim)
> + array_dimension->print();
> printf("] ");
> }
> }
> diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
> index 92e26bf..0906040 100644
> --- a/src/glsl/ast_function.cpp
> +++ b/src/glsl/ast_function.cpp
> @@ -870,6 +870,7 @@ process_array_constructor(exec_list *instructions,
> }
>
> bool all_parameters_are_constant = true;
> + const glsl_type *element_type = constructor_type->fields.array;
>
> /* Type cast each parameter and, if possible, fold constants. */
> foreach_in_list_safe(ir_rvalue, ir, &actual_parameters) {
> @@ -896,12 +897,34 @@ process_array_constructor(exec_list *instructions,
> }
> }
>
> - if (result->type != constructor_type->fields.array) {
> + if (constructor_type->fields.array->is_unsized_array()) {
> + /* As the inner parameters of the constructor are created without
> + * knowledge of each other we need to check to make sure unsized
> + * parameters of unsized constructors all end up with the same size.
> + *
> + * e.g we make sure to fail for a constructor like this:
> + * vec4[][] a = vec4[][](vec4[](vec4(0.0), vec4(1.0)),
> + * vec4[](vec4(0.0), vec4(1.0), vec4(1.0)),
> + * vec4[](vec4(0.0), vec4(1.0)));
> + */
> + if (element_type->is_unsized_array()) {
> + /* This is the first parameter so just get the type */
> + element_type = result->type;
> + } else if (element_type != result->type) {
> + _mesa_glsl_error(loc, state, "type error in array constructor: "
> + "expected: %s, found %s",
> + element_type->name,
> + result->type->name);
> + return ir_rvalue::error_value(ctx);
> + }
> + } else if (result->type != constructor_type->fields.array) {
> _mesa_glsl_error(loc, state, "type error in array constructor: "
> "expected: %s, found %s",
> constructor_type->fields.array->name,
> result->type->name);
> return ir_rvalue::error_value(ctx);
> + } else {
> + element_type = result->type;
> }
>
> /* Attempt to convert the parameter to a constant valued expression.
> @@ -918,6 +941,14 @@ process_array_constructor(exec_list *instructions,
> ir->replace_with(result);
> }
>
> + if (constructor_type->fields.array->is_unsized_array()) {
> + constructor_type =
> + glsl_type::get_array_instance(element_type,
> + parameter_count);
> + assert(constructor_type != NULL);
> + assert(constructor_type->length == parameter_count);
> + }
> +
> if (all_parameters_are_constant)
> return new(ctx) ir_constant(constructor_type, &actual_parameters);
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index de13060..6d2dc2e 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -677,8 +677,30 @@ validate_assignment(struct _mesa_glsl_parse_state *state,
> * Note: Whole-array assignments are not permitted in GLSL 1.10, but this
> * is handled by ir_dereference::is_lvalue.
> */
> - if (lhs_type->is_unsized_array() && rhs->type->is_array()
> - && (lhs_type->fields.array == rhs->type->fields.array)) {
> + const glsl_type *lhs_t = lhs_type;
> + const glsl_type *rhs_t = rhs->type;
> + bool unsized_array = false;
> + while(lhs_t->is_array()) {
> + if (rhs_t == lhs_t)
> + break; /* the rest of the inner arrays match so break out early */
> + if (!rhs_t->is_array()) {
> + unsized_array = false;
> + break; /* number of dimensions mismatch */
> + }
> + if (lhs_t->length == rhs_t->length) {
> + lhs_t = lhs_t->fields.array;
> + rhs_t = rhs_t->fields.array;
> + continue;
> + } else if (lhs_t->is_unsized_array()) {
> + unsized_array = true;
> + } else {
> + unsized_array = false;
> + break; /* sized array mismatch */
> + }
> + lhs_t = lhs_t->fields.array;
> + rhs_t = rhs_t->fields.array;
> + }
> + if (unsized_array) {
> if (is_initializer) {
> return rhs;
> } else {
> @@ -1682,6 +1704,10 @@ ast_expression::do_hir(exec_list *instructions,
> break;
> }
>
> + case ast_unsized_array_dim:
> + assert(!"ast_unsized_array_dim: Should never get here.");
> + break;
> +
> case ast_function_call:
> /* Should *NEVER* get here. ast_function_call should always be handled
> * by ast_function_expression::hir.
> @@ -1845,6 +1871,14 @@ process_array_size(exec_node *node,
> exec_list dummy_instructions;
>
> ast_node *array_size = exec_node_data(ast_node, node, link);
> +
> + /**
> + * Dimensions other than the outermost dimension can by unsized if they
> + * are immediately sized by a constructor or initializer.
> + */
> + if (((ast_expression*)array_size)->oper == ast_unsized_array_dim)
> + return 0;
> +
> ir_rvalue *const ir = array_size->hir(& dummy_instructions, state);
> YYLTYPE loc = array_size->get_location();
>
> @@ -1913,14 +1947,6 @@ process_array_type(YYLTYPE *loc, const glsl_type *base,
> base->name);
> return glsl_type::error_type;
> }
> -
> - if (base->length == 0) {
> - _mesa_glsl_error(loc, state,
> - "only the outermost array dimension can "
> - "be unsized",
> - base->name);
> - return glsl_type::error_type;
> - }
> }
>
> for (exec_node *node = array_specifier->array_dimensions.tail_pred;
> @@ -1928,9 +1954,6 @@ process_array_type(YYLTYPE *loc, const glsl_type *base,
> unsigned array_size = process_array_size(node, state);
> array_type = glsl_type::get_array_instance(array_type, array_size);
> }
> -
> - if (array_specifier->is_unsized_array)
> - array_type = glsl_type::get_array_instance(array_type, 0);
> }
>
> return array_type;
> @@ -2407,6 +2430,25 @@ is_conflicting_fragcoord_redeclaration(struct _mesa_glsl_parse_state *state,
> return false;
> }
>
> +static inline void
> +validate_array_dimensions(const glsl_type *t,
> + struct _mesa_glsl_parse_state *state,
> + YYLTYPE *loc) {
> + if (t->is_array()) {
> + t = t->fields.array;
> + while (t->is_array()) {
> + if (t->is_unsized_array()) {
> + _mesa_glsl_error(loc, state,
> + "only the outermost array dimension can "
> + "be unsized",
> + t->name);
> + break;
> + }
> + t = t->fields.array;
> + }
> + }
> +}
> +
> static void
> apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
> ir_variable *var,
> @@ -3927,6 +3969,8 @@ ast_declarator_list::hir(exec_list *instructions,
> result = process_initializer((earlier == NULL) ? var : earlier,
> decl, this->type,
> &initializer_instructions, state);
> + } else {
> + validate_array_dimensions(var_type, state, &loc);
> }
>
> /* From page 23 (page 29 of the PDF) of the GLSL 1.10 spec:
> @@ -5384,6 +5428,7 @@ ast_process_structure_or_interface_block(exec_list *instructions,
>
> field_type = process_array_type(&loc, decl_type,
> decl->array_specifier, state);
> + validate_array_dimensions(field_type, state, &loc);
> fields[i].type = field_type;
> fields[i].name = decl->identifier;
> fields[i].location = -1;
> @@ -5810,6 +5855,9 @@ ast_interface_block::hir(exec_list *instructions,
> ir_variable *var;
>
> if (this->array_specifier != NULL) {
> + const glsl_type *block_array_type =
> + process_array_type(&loc, block_type, this->array_specifier, state);
> +
> /* Section 4.3.7 (Interface Blocks) of the GLSL 1.50 spec says:
> *
> * For uniform blocks declared an array, each individual array
> @@ -5829,16 +5877,14 @@ ast_interface_block::hir(exec_list *instructions,
> * interface array size *doesn't* need to be specified is on a
> * geometry shader input.
> */
> - if (this->array_specifier->is_unsized_array &&
> + if (block_array_type->is_unsized_array() &&
> (state->stage != MESA_SHADER_GEOMETRY || !this->layout.flags.q.in)) {
> _mesa_glsl_error(&loc, state,
> "only geometry shader inputs may be unsized "
> "instance block arrays");
>
> }
> -
> - const glsl_type *block_array_type =
> - process_array_type(&loc, block_type, this->array_specifier, state);
> +//TODO: make sure no other array dimensions are unsized arrays ??
Just noticed I left this comment in, will remove it. I will need to add
extra validation here for desktop support but not for the ES as
interface blocks can't be AoA in ES.
>
> /* From section 4.3.9 (Interface Blocks) of the GLSL ES 3.10 spec:
> *
> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
> index 3ce9e10..1f08893 100644
> --- a/src/glsl/glsl_parser.yy
> +++ b/src/glsl/glsl_parser.yy
> @@ -1840,7 +1840,9 @@ array_specifier:
> '[' ']'
> {
> void *ctx = state;
> - $$ = new(ctx) ast_array_specifier(@1);
> + $$ = new(ctx) ast_array_specifier(@1, new(ctx) ast_expression(
> + ast_unsized_array_dim, NULL,
> + NULL, NULL));
> $$->set_location_range(@1, @2);
> }
> | '[' constant_expression ']'
> @@ -1851,17 +1853,16 @@ array_specifier:
> }
> | array_specifier '[' ']'
> {
> + void *ctx = state;
> $$ = $1;
>
> if (!state->ARB_arrays_of_arrays_enable) {
> _mesa_glsl_error(& @1, state,
> "GL_ARB_arrays_of_arrays "
> "required for defining arrays of arrays");
> - } else {
> - _mesa_glsl_error(& @1, state,
> - "only the outermost array dimension can "
> - "be unsized");
> }
> + $$->add_dimension(new(ctx) ast_expression(ast_unsized_array_dim, NULL,
> + NULL, NULL));
> }
> | array_specifier '[' constant_expression ']'
> {
More information about the mesa-dev
mailing list