[Mesa-dev] [PATCH V7 03/24] glsl: allow AoA to be sized by initializer or constructor
Samuel Iglesias Gonsálvez
siglesias at igalia.com
Thu Oct 8 02:11:58 PDT 2015
On 08/10/15 11:08, Samuel Iglesias Gonsálvez wrote:
> On 07/10/15 00:47, 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 | 15 ++-------
>> src/glsl/ast_array_index.cpp | 7 ++--
>> src/glsl/ast_function.cpp | 33 +++++++++++++++++-
>> src/glsl/ast_to_hir.cpp | 79 ++++++++++++++++++++++++++++++++++----------
>> src/glsl/glsl_parser.yy | 11 +++---
>> 5 files changed, 104 insertions(+), 41 deletions(-)
>>
>> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
>> index 4c31436..b43be24 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,
>>
>> @@ -318,16 +319,7 @@ public:
>>
>> 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);
>> @@ -340,11 +332,8 @@ public:
>>
>> 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 5e8f49d..7855e0a 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 26d4c62..cf4e64a 100644
>> --- a/src/glsl/ast_function.cpp
>> +++ b/src/glsl/ast_function.cpp
>> @@ -950,6 +950,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) {
>> @@ -976,12 +977,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.
>> @@ -998,6 +1021,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 9511440..3dfa3e6 100644
>> --- a/src/glsl/ast_to_hir.cpp
>> +++ b/src/glsl/ast_to_hir.cpp
>> @@ -782,8 +782,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 {
>> @@ -1804,6 +1826,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.
>> @@ -1967,6 +1993,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
>
> s/by/be
>
>> + * 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();
>>
>> @@ -2035,14 +2069,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;
>> @@ -2050,9 +2076,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;
>> @@ -2591,6 +2614,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;
>> + }
>> + }
>> +}
>> +
>
> It seems the changes related to of array dimensions could be in a
^ validation
> separate patch.
>
>> static void
>> apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
>> ir_variable *var,
>> @@ -4264,6 +4306,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:
>> @@ -5789,6 +5833,7 @@ ast_process_structure_or_interface_block(exec_list *instructions,
>>
>> const struct glsl_type *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;
>> @@ -6304,6 +6349,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
>> @@ -6327,7 +6375,7 @@ ast_interface_block::hir(exec_list *instructions,
>> * tessellation control shader output, and tessellation evaluation
>> * shader input.
>> */
>> - if (this->array_specifier->is_unsized_array) {
>> + if (block_array_type->is_unsized_array()) {
>
>
> I guess this is needed as interface blocks (e.g. uniform blocks) can be
> arrays of arrays?
>
> If that it's the case, it seems this hunk can be in a separate patch, right?
>
> Sam
>
>> bool allow_inputs = state->stage == MESA_SHADER_GEOMETRY ||
>> state->stage == MESA_SHADER_TESS_CTRL ||
>> state->stage == MESA_SHADER_TESS_EVAL;
>> @@ -6354,9 +6402,6 @@ ast_interface_block::hir(exec_list *instructions,
>> }
>> }
>>
>> - const glsl_type *block_array_type =
>> - process_array_type(&loc, block_type, this->array_specifier, state);
>> -
>> /* From section 4.3.9 (Interface Blocks) of the GLSL ES 3.10 spec:
>> *
>> * * Arrays of arrays of blocks are not allowed
>> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
>> index c1bcccc..16c9171 100644
>> --- a/src/glsl/glsl_parser.yy
>> +++ b/src/glsl/glsl_parser.yy
>> @@ -1962,7 +1962,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 ']'
>> @@ -1973,17 +1975,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 ']'
>> {
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list