[Mesa-dev] [PATCH 3/8] glsl: stop copying struct and interface member names
Thomas Helland
thomashelland90 at gmail.com
Wed Aug 9 07:25:44 UTC 2017
I thought I sent out a reply to this yesterday. Apparently I was too
tired to remember... I was thinking a helper function to acquire the
name would be nice, but there is not that many uses, so we probably
should not bother. This patch is:
Reviewed-by: Thomas Helland<thomashelland90 at gmail.com>
2017-08-09 5:34 GMT+02:00 Timothy Arceri <tarceri at itsqueeze.com>:
> We are currently copying the name for each member dereference
> but we can just share a single instance of the string provided
> by the type.
>
> This change also stops us recalculating the field index
> repeatedly.
> ---
> src/compiler/glsl/ast_array_index.cpp | 14 ++++-----
> src/compiler/glsl/glsl_to_nir.cpp | 2 +-
> src/compiler/glsl/ir.cpp | 8 ++---
> src/compiler/glsl/ir.h | 4 +--
> src/compiler/glsl/ir_clone.cpp | 4 ++-
> src/compiler/glsl/ir_constant_expression.cpp | 4 +--
> src/compiler/glsl/ir_print_visitor.cpp | 5 +++-
> src/compiler/glsl/linker.cpp | 9 +-----
> src/compiler/glsl/lower_buffer_access.cpp | 6 ++--
> src/compiler/glsl/lower_named_interface_blocks.cpp | 3 +-
> src/compiler/glsl/opt_structure_splitting.cpp | 10 ++-----
> src/mesa/program/ir_to_mesa.cpp | 6 ++--
> src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 34 ++++++++++------------
> 13 files changed, 50 insertions(+), 59 deletions(-)
>
> diff --git a/src/compiler/glsl/ast_array_index.cpp b/src/compiler/glsl/ast_array_index.cpp
> index f6b7a64a28..efddbed6ea 100644
> --- a/src/compiler/glsl/ast_array_index.cpp
> +++ b/src/compiler/glsl/ast_array_index.cpp
> @@ -81,37 +81,37 @@ update_max_array_access(ir_rvalue *ir, int idx, YYLTYPE *loc,
> while (deref_array != NULL) {
> deref_array_prev = deref_array;
> deref_array = deref_array->array->as_dereference_array();
> }
> if (deref_array_prev != NULL)
> deref_var = deref_array_prev->array->as_dereference_variable();
> }
>
> if (deref_var != NULL) {
> if (deref_var->var->is_interface_instance()) {
> - unsigned field_index =
> - deref_record->record->type->field_index(deref_record->field);
> - assert(field_index < deref_var->var->get_interface_type()->length);
> + unsigned field_idx = deref_record->field_idx;
> + assert(field_idx < deref_var->var->get_interface_type()->length);
>
> int *const max_ifc_array_access =
> deref_var->var->get_max_ifc_array_access();
>
> assert(max_ifc_array_access != NULL);
>
> - if (idx > max_ifc_array_access[field_index]) {
> - max_ifc_array_access[field_index] = idx;
> + if (idx > max_ifc_array_access[field_idx]) {
> + max_ifc_array_access[field_idx] = idx;
>
> /* Check whether this access will, as a side effect, implicitly
> * cause the size of a built-in array to be too large.
> */
> - check_builtin_array_max_size(deref_record->field, idx+1, *loc,
> - state);
> + const char *field_name =
> + deref_record->record->type->fields.structure[field_idx].name;
> + check_builtin_array_max_size(field_name, idx+1, *loc, state);
> }
> }
> }
> }
> }
>
>
> static int
> get_implicit_array_size(struct _mesa_glsl_parse_state *state,
> ir_rvalue *array)
> diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp
> index e5166855e8..bb2ba17b22 100644
> --- a/src/compiler/glsl/glsl_to_nir.cpp
> +++ b/src/compiler/glsl/glsl_to_nir.cpp
> @@ -2198,21 +2198,21 @@ nir_visitor::visit(ir_dereference_variable *ir)
> nir_deref_var *deref = nir_deref_var_create(this->shader, var);
> this->deref_head = deref;
> this->deref_tail = &deref->deref;
> }
>
> void
> nir_visitor::visit(ir_dereference_record *ir)
> {
> ir->record->accept(this);
>
> - int field_index = this->deref_tail->type->field_index(ir->field);
> + int field_index = ir->field_idx;
> assert(field_index >= 0);
>
> nir_deref_struct *deref = nir_deref_struct_create(this->deref_tail, field_index);
> deref->deref.type = ir->type;
> this->deref_tail->child = &deref->deref;
> this->deref_tail = &deref->deref;
> }
>
> void
> nir_visitor::visit(ir_dereference_array *ir)
> diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
> index 98bbd91539..52ca83689e 100644
> --- a/src/compiler/glsl/ir.cpp
> +++ b/src/compiler/glsl/ir.cpp
> @@ -1097,24 +1097,22 @@ ir_constant::get_array_element(unsigned i) const
> */
> if (int(i) < 0)
> i = 0;
> else if (i >= this->type->length)
> i = this->type->length - 1;
>
> return array_elements[i];
> }
>
> ir_constant *
> -ir_constant::get_record_field(const char *name)
> +ir_constant::get_record_field(int idx)
> {
> - int idx = this->type->field_index(name);
> -
> if (idx < 0)
> return NULL;
>
> if (this->components.is_empty())
> return NULL;
>
> exec_node *node = this->components.get_head_raw();
> for (int i = 0; i < idx; i++) {
> node = node->next;
>
> @@ -1445,34 +1443,34 @@ ir_dereference_array::set_array(ir_rvalue *value)
> }
>
>
> ir_dereference_record::ir_dereference_record(ir_rvalue *value,
> const char *field)
> : ir_dereference(ir_type_dereference_record)
> {
> assert(value != NULL);
>
> this->record = value;
> - this->field = ralloc_strdup(this, field);
> this->type = this->record->type->field_type(field);
> + this->field_idx = this->record->type->field_index(field);
> }
>
>
> ir_dereference_record::ir_dereference_record(ir_variable *var,
> const char *field)
> : ir_dereference(ir_type_dereference_record)
> {
> void *ctx = ralloc_parent(var);
>
> this->record = new(ctx) ir_dereference_variable(var);
> - this->field = ralloc_strdup(this, field);
> this->type = this->record->type->field_type(field);
> + this->field_idx = this->record->type->field_index(field);
> }
>
> bool
> ir_dereference::is_lvalue(const struct _mesa_glsl_parse_state *state) const
> {
> ir_variable *var = this->variable_referenced();
>
> /* Every l-value derference chain eventually ends in a variable.
> */
> if ((var == NULL) || var->data.read_only)
> diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
> index d53b44304d..ce4ade9e80 100644
> --- a/src/compiler/glsl/ir.h
> +++ b/src/compiler/glsl/ir.h
> @@ -2101,21 +2101,21 @@ public:
> }
>
> virtual void accept(ir_visitor *v)
> {
> v->visit(this);
> }
>
> virtual ir_visitor_status accept(ir_hierarchical_visitor *);
>
> ir_rvalue *record;
> - const char *field;
> + int field_idx;
> };
>
>
> /**
> * Data stored in an ir_constant
> */
> union ir_constant_data {
> unsigned u[16];
> int i[16];
> float f[16];
> @@ -2185,21 +2185,21 @@ public:
> float get_float_component(unsigned i) const;
> double get_double_component(unsigned i) const;
> int get_int_component(unsigned i) const;
> unsigned get_uint_component(unsigned i) const;
> int64_t get_int64_component(unsigned i) const;
> uint64_t get_uint64_component(unsigned i) const;
> /*@}*/
>
> ir_constant *get_array_element(unsigned i) const;
>
> - ir_constant *get_record_field(const char *name);
> + ir_constant *get_record_field(int idx);
>
> /**
> * Copy the values on another constant at a given offset.
> *
> * The offset is ignored for array or struct copies, it's only for
> * scalars or vectors into vectors or matrices.
> *
> * With identical types on both sides and zero offset it's clone()
> * without creating a new object.
> */
> diff --git a/src/compiler/glsl/ir_clone.cpp b/src/compiler/glsl/ir_clone.cpp
> index 941e0865cb..4fb9fa31f9 100644
> --- a/src/compiler/glsl/ir_clone.cpp
> +++ b/src/compiler/glsl/ir_clone.cpp
> @@ -187,22 +187,24 @@ ir_dereference_array *
> ir_dereference_array::clone(void *mem_ctx, struct hash_table *ht) const
> {
> return new(mem_ctx) ir_dereference_array(this->array->clone(mem_ctx, ht),
> this->array_index->clone(mem_ctx,
> ht));
> }
>
> ir_dereference_record *
> ir_dereference_record::clone(void *mem_ctx, struct hash_table *ht) const
> {
> + const char *field_name =
> + this->record->type->fields.structure[this->field_idx].name;
> return new(mem_ctx) ir_dereference_record(this->record->clone(mem_ctx, ht),
> - this->field);
> + field_name);
> }
>
> ir_texture *
> ir_texture::clone(void *mem_ctx, struct hash_table *ht) const
> {
> ir_texture *new_tex = new(mem_ctx) ir_texture(this->op);
> new_tex->type = this->type;
>
> new_tex->sampler = this->sampler->clone(mem_ctx, ht);
> if (this->coordinate)
> diff --git a/src/compiler/glsl/ir_constant_expression.cpp b/src/compiler/glsl/ir_constant_expression.cpp
> index d4a8b7d020..a493657d35 100644
> --- a/src/compiler/glsl/ir_constant_expression.cpp
> +++ b/src/compiler/glsl/ir_constant_expression.cpp
> @@ -478,21 +478,21 @@ constant_referenced(const ir_dereference *deref,
> ir_constant *substore;
> int suboffset;
>
> if (!constant_referenced(deref, variable_context, substore, suboffset))
> break;
>
> /* Since we're dropping it on the floor...
> */
> assert(suboffset == 0);
>
> - store = substore->get_record_field(dr->field);
> + store = substore->get_record_field(dr->field_idx);
> break;
> }
>
> case ir_type_dereference_variable: {
> const ir_dereference_variable *const dv =
> (const ir_dereference_variable *) deref;
>
> hash_entry *entry = _mesa_hash_table_search(variable_context, dv->var);
> if (entry)
> store = (ir_constant *) entry->data;
> @@ -821,21 +821,21 @@ ir_dereference_array::constant_expression_value(struct hash_table *variable_cont
> }
> return NULL;
> }
>
>
> ir_constant *
> ir_dereference_record::constant_expression_value(struct hash_table *)
> {
> ir_constant *v = this->record->constant_expression_value();
>
> - return (v != NULL) ? v->get_record_field(this->field) : NULL;
> + return (v != NULL) ? v->get_record_field(this->field_idx) : NULL;
> }
>
>
> ir_constant *
> ir_assignment::constant_expression_value(struct hash_table *)
> {
> /* FINISHME: Handle CEs involving assignment (return RHS) */
> return NULL;
> }
>
> diff --git a/src/compiler/glsl/ir_print_visitor.cpp b/src/compiler/glsl/ir_print_visitor.cpp
> index a32a410919..64aeaee286 100644
> --- a/src/compiler/glsl/ir_print_visitor.cpp
> +++ b/src/compiler/glsl/ir_print_visitor.cpp
> @@ -416,21 +416,24 @@ void ir_print_visitor::visit(ir_dereference_array *ir)
> ir->array->accept(this);
> ir->array_index->accept(this);
> fprintf(f, ") ");
> }
>
>
> void ir_print_visitor::visit(ir_dereference_record *ir)
> {
> fprintf(f, "(record_ref ");
> ir->record->accept(this);
> - fprintf(f, " %s) ", ir->field);
> +
> + const char *field_name =
> + ir->record->type->fields.structure[ir->field_idx].name;
> + fprintf(f, " %s) ", field_name);
> }
>
>
> void ir_print_visitor::visit(ir_assignment *ir)
> {
> fprintf(f, "(assign ");
>
> if (ir->condition)
> ir->condition->accept(this);
>
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index b4784c5119..5f22eb36ae 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -241,28 +241,21 @@ public:
> virtual ir_visitor_status visit_leave(ir_dereference_array *ir)
> {
> const glsl_type *const vt = ir->array->type;
> if (vt->is_array())
> ir->type = vt->fields.array;
> return visit_continue;
> }
>
> virtual ir_visitor_status visit_leave(ir_dereference_record *ir)
> {
> - for (unsigned i = 0; i < ir->record->type->length; i++) {
> - const struct glsl_struct_field *field =
> - &ir->record->type->fields.structure[i];
> - if (strcmp(field->name, ir->field) == 0) {
> - ir->type = field->type;
> - break;
> - }
> - }
> + ir->type = ir->record->type->fields.structure[ir->field_idx].type;
> return visit_continue;
> }
> };
>
>
> class array_resize_visitor : public deref_type_updater {
> public:
> unsigned num_vertices;
> gl_shader_program *prog;
> gl_shader_stage stage;
> diff --git a/src/compiler/glsl/lower_buffer_access.cpp b/src/compiler/glsl/lower_buffer_access.cpp
> index 24a96e2fba..76d366c4b9 100644
> --- a/src/compiler/glsl/lower_buffer_access.cpp
> +++ b/src/compiler/glsl/lower_buffer_access.cpp
> @@ -245,21 +245,21 @@ lower_buffer_access::is_dereferenced_thing_row_major(const ir_rvalue *deref)
> ir = array_deref->array;
> break;
> }
>
> case ir_type_dereference_record: {
> const ir_dereference_record *const record_deref =
> (const ir_dereference_record *) ir;
>
> ir = record_deref->record;
>
> - const int idx = ir->type->field_index(record_deref->field);
> + const int idx = record_deref->field_idx;
> assert(idx >= 0);
>
> const enum glsl_matrix_layout matrix_layout =
> glsl_matrix_layout(ir->type->fields.structure[idx].matrix_layout);
>
> switch (matrix_layout) {
> case GLSL_MATRIX_LAYOUT_INHERITED:
> break;
> case GLSL_MATRIX_LAYOUT_COLUMN_MAJOR:
> return false;
> @@ -438,22 +438,22 @@ lower_buffer_access::setup_buffer_access(void *mem_ctx,
> field_align = type->std430_base_alignment(field_row_major);
> else
> field_align = type->std140_base_alignment(field_row_major);
>
> if (struct_type->fields.structure[i].offset != -1) {
> intra_struct_offset = struct_type->fields.structure[i].offset;
> }
>
> intra_struct_offset = glsl_align(intra_struct_offset, field_align);
>
> - if (strcmp(struct_type->fields.structure[i].name,
> - deref_record->field) == 0) {
> + assert(deref_record->field_idx >= 0);
> + if (i == (unsigned) deref_record->field_idx) {
> if (struct_field)
> *struct_field = &struct_type->fields.structure[i];
> break;
> }
>
> if (packing == GLSL_INTERFACE_PACKING_STD430)
> intra_struct_offset += type->std430_size(field_row_major);
> else
> intra_struct_offset += type->std140_size(field_row_major);
>
> diff --git a/src/compiler/glsl/lower_named_interface_blocks.cpp b/src/compiler/glsl/lower_named_interface_blocks.cpp
> index a00e60dd77..064694128b 100644
> --- a/src/compiler/glsl/lower_named_interface_blocks.cpp
> +++ b/src/compiler/glsl/lower_named_interface_blocks.cpp
> @@ -260,21 +260,22 @@ flatten_named_interface_blocks_declarations::handle_rvalue(ir_rvalue **rvalue)
> * support code.
> */
> if (var->data.mode == ir_var_uniform || var->data.mode == ir_var_shader_storage)
> return;
>
> if (var->get_interface_type() != NULL) {
> char *iface_field_name =
> ralloc_asprintf(mem_ctx, "%s %s.%s.%s",
> var->data.mode == ir_var_shader_in ? "in" : "out",
> var->get_interface_type()->name,
> - var->name, ir->field);
> + var->name,
> + ir->record->type->fields.structure[ir->field_idx].name);
>
> /* Find the variable in the set of flattened interface blocks */
> hash_entry *entry = _mesa_hash_table_search(interface_namespace,
> iface_field_name);
> assert(entry);
> ir_variable *found_var = (ir_variable *) entry->data;
>
> ir_dereference_variable *deref_var =
> new(mem_ctx) ir_dereference_variable(found_var);
>
> diff --git a/src/compiler/glsl/opt_structure_splitting.cpp b/src/compiler/glsl/opt_structure_splitting.cpp
> index 8439430387..a015d6d7c9 100644
> --- a/src/compiler/glsl/opt_structure_splitting.cpp
> +++ b/src/compiler/glsl/opt_structure_splitting.cpp
> @@ -226,27 +226,23 @@ ir_structure_splitting_visitor::split_deref(ir_dereference **deref)
>
> ir_dereference_record *deref_record = (ir_dereference_record *)*deref;
> ir_dereference_variable *deref_var = deref_record->record->as_dereference_variable();
> if (!deref_var)
> return;
>
> variable_entry *entry = get_splitting_entry(deref_var->var);
> if (!entry)
> return;
>
> - unsigned int i;
> - for (i = 0; i < entry->var->type->length; i++) {
> - if (strcmp(deref_record->field,
> - entry->var->type->fields.structure[i].name) == 0)
> - break;
> - }
> - assert(i != entry->var->type->length);
> + int i = deref_record->field_idx;
> + assert(i >= 0);
> + assert((unsigned) i < entry->var->type->length);
>
> *deref = new(entry->mem_ctx) ir_dereference_variable(entry->components[i]);
> }
>
> void
> ir_structure_splitting_visitor::handle_rvalue(ir_rvalue **rvalue)
> {
> if (!*rvalue)
> return;
>
> diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
> index db7f11c6bf..96b06621b5 100644
> --- a/src/mesa/program/ir_to_mesa.cpp
> +++ b/src/mesa/program/ir_to_mesa.cpp
> @@ -1595,22 +1595,23 @@ ir_to_mesa_visitor::visit(ir_dereference_array *ir)
>
> void
> ir_to_mesa_visitor::visit(ir_dereference_record *ir)
> {
> unsigned int i;
> const glsl_type *struct_type = ir->record->type;
> int offset = 0;
>
> ir->record->accept(this);
>
> + assert(ir->field_idx >= 0);
> for (i = 0; i < struct_type->length; i++) {
> - if (strcmp(struct_type->fields.structure[i].name, ir->field) == 0)
> + if (i == (unsigned) ir->field_idx)
> break;
> offset += type_size(struct_type->fields.structure[i].type);
> }
>
> /* If the type is smaller than a vec4, replicate the last channel out. */
> if (ir->type->is_scalar() || ir->type->is_vector())
> this->result.swizzle = swizzle_for_size(ir->type->vector_elements);
> else
> this->result.swizzle = SWIZZLE_NOOP;
>
> @@ -1677,22 +1678,21 @@ calc_sampler_offsets(struct gl_shader_program *prog, ir_dereference *deref,
>
> *array_elements *= deref_arr->array->type->length;
>
> calc_sampler_offsets(prog, deref_arr->array->as_dereference(),
> offset, array_elements, location);
> break;
> }
>
> case ir_type_dereference_record: {
> ir_dereference_record *deref_record = deref->as_dereference_record();
> - unsigned field_index =
> - deref_record->record->type->field_index(deref_record->field);
> + unsigned field_index = deref_record->field_idx;
> *location +=
> deref_record->record->type->record_location_offset(field_index);
> calc_sampler_offsets(prog, deref_record->record->as_dereference(),
> offset, array_elements, location);
> break;
> }
>
> default:
> unreachable("Invalid deref type");
> break;
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index aaa5cddcf3..bada7f4ea8 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -2920,22 +2920,23 @@ glsl_to_tgsi_visitor::visit(ir_dereference_array *ir)
>
> void
> glsl_to_tgsi_visitor::visit(ir_dereference_record *ir)
> {
> unsigned int i;
> const glsl_type *struct_type = ir->record->type;
> int offset = 0;
>
> ir->record->accept(this);
>
> + assert(ir->field_idx >= 0);
> for (i = 0; i < struct_type->length; i++) {
> - if (strcmp(struct_type->fields.structure[i].name, ir->field) == 0)
> + if (i == (unsigned) ir->field_idx)
> break;
> offset += type_size(struct_type->fields.structure[i].type);
> }
>
> /* If the type is smaller than a vec4, replicate the last channel out. */
> if (ir->type->is_scalar() || ir->type->is_vector())
> this->result.swizzle = swizzle_for_size(ir->type->vector_elements);
> else
> this->result.swizzle = SWIZZLE_NOOP;
>
> @@ -3777,37 +3778,34 @@ glsl_to_tgsi_visitor::visit_shared_intrinsic(ir_call *ir)
>
> static void
> get_image_qualifiers(ir_dereference *ir, const glsl_type **type,
> bool *memory_coherent, bool *memory_volatile,
> bool *memory_restrict, unsigned *image_format)
> {
>
> switch (ir->ir_type) {
> case ir_type_dereference_record: {
> ir_dereference_record *deref_record = ir->as_dereference_record();
> - const glsl_type *struct_type = deref_record->record->type;
>
> - for (unsigned i = 0; i < struct_type->length; i++) {
> - if (!strcmp(struct_type->fields.structure[i].name,
> - deref_record->field)) {
> - *type = struct_type->fields.structure[i].type->without_array();
> - *memory_coherent =
> - struct_type->fields.structure[i].memory_coherent;
> - *memory_volatile =
> - struct_type->fields.structure[i].memory_volatile;
> - *memory_restrict =
> - struct_type->fields.structure[i].memory_restrict;
> - *image_format =
> - struct_type->fields.structure[i].image_format;
> - break;
> - }
> - }
> + *type = deref_record->type;
> +
> + const glsl_type *struct_type =
> + deref_record->record->type->without_array();
> + int fild_idx = deref_record->field_idx;
> + *memory_coherent =
> + struct_type->fields.structure[fild_idx].memory_coherent;
> + *memory_volatile =
> + struct_type->fields.structure[fild_idx].memory_volatile;
> + *memory_restrict =
> + struct_type->fields.structure[fild_idx].memory_restrict;
> + *image_format =
> + struct_type->fields.structure[fild_idx].image_format;
> break;
> }
>
> case ir_type_dereference_array: {
> ir_dereference_array *deref_arr = ir->as_dereference_array();
> get_image_qualifiers((ir_dereference *)deref_arr->array, type,
> memory_coherent, memory_volatile, memory_restrict,
> image_format);
> break;
> }
> @@ -4121,21 +4119,21 @@ void
> glsl_to_tgsi_visitor::calc_deref_offsets(ir_dereference *tail,
> unsigned *array_elements,
> uint16_t *index,
> st_src_reg *indirect,
> unsigned *location)
> {
> switch (tail->ir_type) {
> case ir_type_dereference_record: {
> ir_dereference_record *deref_record = tail->as_dereference_record();
> const glsl_type *struct_type = deref_record->record->type;
> - int field_index = deref_record->record->type->field_index(deref_record->field);
> + int field_index = deref_record->field_idx;
>
> calc_deref_offsets(deref_record->record->as_dereference(), array_elements, index, indirect, location);
>
> assert(field_index >= 0);
> *location += struct_type->record_location_offset(field_index);
> break;
> }
>
> case ir_type_dereference_array: {
> ir_dereference_array *deref_arr = tail->as_dereference_array();
> --
> 2.13.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list