[Mesa-dev] [PATCH] glsl: remove element_type() helper
Timothy Arceri
t_arceri at yahoo.com.au
Thu Apr 30 15:48:10 PDT 2015
I've made two small changes to this locally see below.
On Thu, 2015-04-30 at 21:31 +1000, Timothy Arceri wrote:
> We now have is_array() and without_array() that make the
> code much clearer and remove the need for this.
>
> For all remaining calls to this we already knew that
> the type was an array so it wasn't adding any value.
> ---
> src/glsl/ast_array_index.cpp | 2 +-
> src/glsl/ast_function.cpp | 8 ++++----
> src/glsl/ast_to_hir.cpp | 8 ++++----
> src/glsl/glsl_parser_extras.cpp | 4 ++--
> src/glsl/glsl_types.cpp | 2 +-
> src/glsl/glsl_types.h | 14 +-------------
> src/glsl/ir.cpp | 4 ++--
> src/glsl/link_atomics.cpp | 2 +-
> src/glsl/link_varyings.cpp | 2 +-
> src/glsl/linker.cpp | 4 ++--
> src/glsl/lower_clip_distance.cpp | 8 ++++----
> 11 files changed, 23 insertions(+), 35 deletions(-)
>
> diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
> index 481bba8..307b2c8 100644
> --- a/src/glsl/ast_array_index.cpp
> +++ b/src/glsl/ast_array_index.cpp
> @@ -233,7 +233,7 @@ _mesa_ast_array_index_to_hir(void *mem_ctx,
> * values *do* diverge, then the behavior of the operation requiring a
> * dynamically uniform expression is undefined.
> */
> - if (array->type->element_type()->is_sampler()) {
> + if (array->type->fields.array->is_sampler()) {
It makes sense to use without_array() rather than fields.array here for
future AoA support.
> if (!state->is_version(130, 100)) {
> if (state->es_shader) {
> _mesa_glsl_warning(&loc, state,
> diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
> index 87df93e..b5a7efa 100644
> --- a/src/glsl/ast_function.cpp
> +++ b/src/glsl/ast_function.cpp
> @@ -863,7 +863,7 @@ process_array_constructor(exec_list *instructions,
>
> if (is_unsized_array) {
> constructor_type =
> - glsl_type::get_array_instance(constructor_type->element_type(),
> + glsl_type::get_array_instance(constructor_type->fields.array,
> parameter_count);
> assert(constructor_type != NULL);
> assert(constructor_type->length == parameter_count);
> @@ -876,7 +876,7 @@ process_array_constructor(exec_list *instructions,
> ir_rvalue *result = ir;
>
> const glsl_base_type element_base_type =
> - constructor_type->element_type()->base_type;
> + constructor_type->fields.array->base_type;
>
> /* Apply implicit conversions (not the scalar constructor rules!). See
> * the spec quote above. */
> @@ -896,10 +896,10 @@ process_array_constructor(exec_list *instructions,
> }
> }
>
> - if (result->type != constructor_type->element_type()) {
> + if (result->type != constructor_type->fields.array) {
> _mesa_glsl_error(loc, state, "type error in array constructor: "
> "expected: %s, found %s",
> - constructor_type->element_type()->name,
> + constructor_type->fields.array->name,
> result->type->name);
> return ir_rvalue::error_value(ctx);
> }
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 543ebd8..7374bbb 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -678,7 +678,7 @@ validate_assignment(struct _mesa_glsl_parse_state *state,
> * is handled by ir_dereference::is_lvalue.
> */
> if (lhs_type->is_unsized_array() && rhs->type->is_array()
> - && (lhs_type->element_type() == rhs->type->element_type())) {
> + && (lhs_type->fields.array == rhs->type->fields.array)) {
> if (is_initializer) {
> return rhs;
> } else {
> @@ -820,7 +820,7 @@ do_assignment(exec_list *instructions, struct _mesa_glsl_parse_state *state,
> var->data.max_array_access);
> }
>
> - var->type = glsl_type::get_array_instance(lhs->type->element_type(),
> + var->type = glsl_type::get_array_instance(lhs->type->fields.array,
> rhs->type->array_size());
> d->type = var->type;
> }
> @@ -2319,7 +2319,7 @@ apply_image_qualifier_to_variable(const struct ast_type_qualifier *qual,
> YYLTYPE *loc)
> {
> const glsl_type *base_type =
> - (var->type->is_array() ? var->type->element_type() : var->type);
> + (var->type->is_array() ? var->type->fields.array : var->type);
>
> if (base_type->is_image()) {
> if (var->data.mode != ir_var_uniform &&
> @@ -2822,7 +2822,7 @@ get_variable_being_redeclared(ir_variable *var, YYLTYPE loc,
> * type and specify a size."
> */
> if (earlier->type->is_unsized_array() && var->type->is_array()
> - && (var->type->element_type() == earlier->type->element_type())) {
> + && (var->type->fields.array == earlier->type->fields.array)) {
> /* FINISHME: This doesn't match the qualifiers on the two
> * FINISHME: declarations. It's not 100% clear whether this is
> * FINISHME: required or not.
> diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
> index 0aa3c54..f7bc783 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
> @@ -762,7 +762,7 @@ _mesa_ast_set_aggregate_type(const glsl_type *type,
>
> /* If the aggregate is an array, recursively set its elements' types. */
> if (type->is_array()) {
> - /* Each array element has the type type->element_type().
> + /* Each array element has the type type->fields.array.
> *
> * E.g., if <type> if struct S[2] we want to set each element's type to
> * struct S.
> @@ -774,7 +774,7 @@ _mesa_ast_set_aggregate_type(const glsl_type *type,
> link);
>
> if (expr->oper == ast_aggregate)
> - _mesa_ast_set_aggregate_type(type->element_type(), expr);
> + _mesa_ast_set_aggregate_type(type->fields.array, expr);
> }
>
> /* If the aggregate is a struct, recursively set its fields' types. */
> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> index 7b69904..3547561 100644
> --- a/src/glsl/glsl_types.cpp
> +++ b/src/glsl/glsl_types.cpp
> @@ -217,7 +217,7 @@ glsl_type::contains_opaque() const {
> case GLSL_TYPE_ATOMIC_UINT:
> return true;
> case GLSL_TYPE_ARRAY:
> - return element_type()->contains_opaque();
> + return fields.array->contains_opaque();
> case GLSL_TYPE_STRUCT:
> for (unsigned int i = 0; i < length; i++) {
> if (fields.structure[i].type->contains_opaque())
> diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
> index 5645dcd..f54a939 100644
> --- a/src/glsl/glsl_types.h
> +++ b/src/glsl/glsl_types.h
> @@ -228,18 +228,6 @@ struct glsl_type {
> const glsl_type *get_scalar_type() const;
>
> /**
> - * Query the type of elements in an array
> - *
> - * \return
> - * Pointer to the type of elements in the array for array types, or \c NULL
> - * for non-array types.
> - */
> - const glsl_type *element_type() const
> - {
> - return is_array() ? fields.array : NULL;
> - }
> -
> - /**
> * Get the instance of a built-in scalar, vector, or matrix type
> */
> static const glsl_type *get_instance(unsigned base_type, unsigned rows,
> @@ -556,7 +544,7 @@ struct glsl_type {
> if (base_type == GLSL_TYPE_ATOMIC_UINT)
> return ATOMIC_COUNTER_SIZE;
> else if (is_array())
> - return length * element_type()->atomic_size();
> + return length * fields.array->atomic_size();
> else
> return 0;
> }
> diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
> index 4790063..8b54da6 100644
> --- a/src/glsl/ir.cpp
> +++ b/src/glsl/ir.cpp
> @@ -912,7 +912,7 @@ ir_constant::zero(void *mem_ctx, const glsl_type *type)
> c->array_elements = ralloc_array(c, ir_constant *, type->length);
>
> for (unsigned i = 0; i < type->length; i++)
> - c->array_elements[i] = ir_constant::zero(c, type->element_type());
> + c->array_elements[i] = ir_constant::zero(c, type->fields.array);
> }
>
> if (type->is_record()) {
> @@ -1341,7 +1341,7 @@ ir_dereference_array::set_array(ir_rvalue *value)
> const glsl_type *const vt = this->array->type;
>
> if (vt->is_array()) {
> - type = vt->element_type();
> + type = vt->fields.array;
> } else if (vt->is_matrix()) {
> type = vt->column_type();
> } else if (vt->is_vector()) {
> diff --git a/src/glsl/link_atomics.cpp b/src/glsl/link_atomics.cpp
> index 603873a..100d03c 100644
> --- a/src/glsl/link_atomics.cpp
> +++ b/src/glsl/link_atomics.cpp
> @@ -207,7 +207,7 @@ link_assign_atomic_counter_resources(struct gl_context *ctx,
> storage->atomic_buffer_index = i;
> storage->offset = var->data.atomic.offset;
> storage->array_stride = (var->type->is_array() ?
> - var->type->element_type()->atomic_size() : 0);
> + var->type->without_array()->atomic_size() : 0);
> }
>
> /* Assign stage-specific fields. */
> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
> index 4a0ff26..6cb55b3 100644
> --- a/src/glsl/link_varyings.cpp
> +++ b/src/glsl/link_varyings.cpp
> @@ -56,7 +56,7 @@ cross_validate_types_and_qualifiers(struct gl_shader_program *prog,
> const glsl_type *type_to_match = input->type;
> if (consumer_stage == MESA_SHADER_GEOMETRY) {
> assert(type_to_match->is_array()); /* Enforced by ast_to_hir */
> - type_to_match = type_to_match->element_type();
> + type_to_match = type_to_match->fields.array;
> }
> if (type_to_match != output->type) {
> /* There is a bit of a special case for gl_TexCoord. This
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 9693260..c04a225 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -224,7 +224,7 @@ public:
> return visit_continue;
> }
>
> - var->type = glsl_type::get_array_instance(var->type->element_type(),
> + var->type = glsl_type::get_array_instance(var->type->fields.array,
> this->num_vertices);
> var->data.max_array_access = this->num_vertices - 1;
>
> @@ -245,7 +245,7 @@ public:
> {
> const glsl_type *const vt = ir->array->type;
> if (vt->is_array())
> - ir->type = vt->element_type();
> + ir->type = vt->fields.array;
> return visit_continue;
> }
> };
> diff --git a/src/glsl/lower_clip_distance.cpp b/src/glsl/lower_clip_distance.cpp
> index 2d6138d..6dba00c 100644
> --- a/src/glsl/lower_clip_distance.cpp
> +++ b/src/glsl/lower_clip_distance.cpp
> @@ -114,7 +114,7 @@ lower_clip_distance_visitor::visit(ir_variable *ir)
> return visit_continue;
> assert (ir->type->is_array());
>
> - if (!ir->type->element_type()->is_array()) {
> + if (!ir->type->fields.array->is_array()) {
> /* 1D gl_ClipDistance (used for vertex and geometry output, and fragment
> * input).
> */
> @@ -123,7 +123,7 @@ lower_clip_distance_visitor::visit(ir_variable *ir)
>
> this->progress = true;
> this->old_clip_distance_1d_var = ir;
> - assert (ir->type->element_type() == glsl_type::float_type);
> + assert (ir->type->fields.array == glsl_type::float_type);
> unsigned new_size = (ir->type->array_size() + 3) / 4;
>
> /* Clone the old var so that we inherit all of its properties */
> @@ -148,8 +148,8 @@ lower_clip_distance_visitor::visit(ir_variable *ir)
>
> this->progress = true;
> this->old_clip_distance_2d_var = ir;
> - assert (ir->type->element_type()->element_type() == glsl_type::float_type);
> - unsigned new_size = (ir->type->element_type()->array_size() + 3) / 4;
> + assert (ir->type->without_array() == glsl_type::float_type);
I've changed this to ir->type->fields.array->fields.array as this should
only ever be a 2D array.
> + unsigned new_size = (ir->type->fields.array->array_size() + 3) / 4;
>
> /* Clone the old var so that we inherit all of its properties */
> this->new_clip_distance_2d_var = ir->clone(ralloc_parent(ir), NULL);
More information about the mesa-dev
mailing list