[Mesa-dev] [PATCH 13/17] glsl linker: remove interface block instance names
Kenneth Graunke
kenneth at whitecape.org
Wed May 1 13:52:45 PDT 2013
On 04/19/2013 12:35 PM, Jordan Justen wrote:
> Convert interface blocks with instance names into flat
> interface blocks without an instance name.
>
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> ---
> src/glsl/Makefile.sources | 1 +
> src/glsl/ir_optimization.h | 1 +
> src/glsl/linker.cpp | 6 +
> src/glsl/lower_named_interface_blocks.cpp | 202 +++++++++++++++++++++++++++++
> 4 files changed, 210 insertions(+)
> create mode 100644 src/glsl/lower_named_interface_blocks.cpp
Some minor suggestions below. Either way,
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
> index c294aa4..86ae43e 100644
> --- a/src/glsl/Makefile.sources
> +++ b/src/glsl/Makefile.sources
> @@ -62,6 +62,7 @@ LIBGLSL_FILES = \
> $(GLSL_SRCDIR)/lower_mat_op_to_vec.cpp \
> $(GLSL_SRCDIR)/lower_noise.cpp \
> $(GLSL_SRCDIR)/lower_packed_varyings.cpp \
> + $(GLSL_SRCDIR)/lower_named_interface_blocks.cpp \
> $(GLSL_SRCDIR)/lower_packing_builtins.cpp \
> $(GLSL_SRCDIR)/lower_texture_projection.cpp \
> $(GLSL_SRCDIR)/lower_variable_index_to_cond_assign.cpp \
> diff --git a/src/glsl/ir_optimization.h b/src/glsl/ir_optimization.h
> index 2454bbe..c0850ce 100644
> --- a/src/glsl/ir_optimization.h
> +++ b/src/glsl/ir_optimization.h
> @@ -105,6 +105,7 @@ void lower_ubo_reference(struct gl_shader *shader, exec_list *instructions);
> void lower_packed_varyings(void *mem_ctx, unsigned location_base,
> unsigned locations_used, ir_variable_mode mode,
> gl_shader *shader);
> +void lower_named_interface_blocks(void *mem_ctx, gl_shader *shader);
> bool optimize_redundant_jumps(exec_list *instructions);
> bool optimize_split_arrays(exec_list *instructions, bool linked);
>
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 2b30d2b..31339e1 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1733,6 +1733,12 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
> prog->LinkStatus = true;
> }
>
> +
> + for (unsigned int i = 0; i < MESA_SHADER_TYPES; i++) {
> + if (prog->_LinkedShaders[i] != NULL)
> + lower_named_interface_blocks(mem_ctx, prog->_LinkedShaders[i]);
> + }
> +
> /* Implement the GLSL 1.30+ rule for discard vs infinite loops Do
> * it before optimization because we want most of the checks to get
> * dropped thanks to constant propagation.
> diff --git a/src/glsl/lower_named_interface_blocks.cpp b/src/glsl/lower_named_interface_blocks.cpp
> new file mode 100644
> index 0000000..2e0c322
> --- /dev/null
> +++ b/src/glsl/lower_named_interface_blocks.cpp
> @@ -0,0 +1,202 @@
> +/*
> + * Copyright (c) 2013 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +/**
> + * \file lower_named_interface_blocks.cpp
> + *
> + * This lowering pass converts all interface blocks with instance names
> + * into interface blocks without an interface name.
> + *
> + * For example, the following shader:
> + *
> + * out block {
> + * float block_var;
> + * } inst_name;
> + *
> + * main()
> + * {
> + * inst_name.block_var = 0.0;
> + * }
> + *
> + * Is rewritten to:
> + *
> + * out block {
> + * float block_var;
> + * };
> + *
> + * main()
> + * {
> + * block_var = 0.0;
> + * }
> + *
> + * This takes place after the shader code has already been verified with
> + * the interface name in place.
> + *
> + * The linking phase will use the interface name rather than the
> + * interface's instance name.
..."will use the block name rather than"...?
> + *
> + * This modification to the ir allows our currently existing dead code
> + * elimination to work with interface blocks without changes.
> + */
> +
> +#include "glsl_symbol_table.h"
> +#include "ir.h"
> +#include "ir_optimization.h"
> +#include "ir_rvalue_visitor.h"
> +#include "program/hash_table.h"
> +
> +class flatten_named_interface_blocks_declarations : public ir_rvalue_visitor
> +{
> +public:
> + void * const mem_ctx;
> + hash_table *interface_namespace;
> +
> + flatten_named_interface_blocks_declarations(void *mem_ctx)
> + : mem_ctx(mem_ctx)
> + {
> + }
> +
> + void run(exec_list *instructions);
> +
> + virtual ir_visitor_status visit_leave(ir_assignment *);
> + virtual void handle_rvalue(ir_rvalue **rvalue);
> +};
> +
> +void
> +flatten_named_interface_blocks_declarations::run(exec_list *instructions)
> +{
> + interface_namespace = hash_table_ctor(0, hash_table_string_hash,
> + hash_table_string_compare);
> +
> + /* First pass: adjust instance block variables with an instance name
> + * to not have an instance name.
> + *
> + * The interface block variables are stored in the interface_namespace
> + * hash table so they can be used in the second pass.
> + */
> + foreach_list_safe(node, instructions) {
> + ir_variable *var = ((ir_instruction *) node)->as_variable();
> + if (var) {
> + if (!var->is_interface_instance())
> + continue;
Why not write this as:
if (!var || !var->is_interface_instance())
continue;
It's a bit simpler, and also avoids indenting the whole body of the
loop. Not a big deal though.
> +
> + /* It should be possible to handle uniforms during this pass,
> + * but, this will require changes to the other uniform block
> + * support code.
> + */
> + if (var->mode == ir_var_uniform)
> + continue;
> +
> + const glsl_type *const t = var->type;
> + exec_node *insert_pos = var;
> + char *iface_field_name;
> + for (unsigned i = 0; i < t->length; i++) {
> + iface_field_name = ralloc_asprintf(mem_ctx, "%s.%s", t->name,
> + t->fields.structure[i].name);
> +
> + ir_variable *found_var =
> + (ir_variable *) hash_table_find(interface_namespace,
> + iface_field_name);
> + if (!found_var) {
> + ir_variable *new_var =
> + new(mem_ctx) ir_variable(t->fields.structure[i].type,
> + ralloc_strdup(mem_ctx, t->fields.structure[i].name),
> + (ir_variable_mode) var->mode);
> + new_var->interface_type = t;
> + hash_table_insert(interface_namespace, new_var,
> + iface_field_name);
> + insert_pos->insert_after(new_var);
> + insert_pos = new_var;
> + }
> + }
> + var->remove();
> + continue;
This "continue" doesn't do anything, as it's the last statement in the
loop. Better to remove it, for clarity.
> + }
> + }
> +
> + /* Second pass: visit all ir_dereference_record instances, and if they
> + * reference an interface block, then flatten the refererence out.
> + */
> + visit_list_elements(this, instructions);
> + hash_table_dtor(interface_namespace);
> + interface_namespace = NULL;
> +}
> +
> +ir_visitor_status
> +flatten_named_interface_blocks_declarations::visit_leave(ir_assignment *ir)
> +{
> + ir_dereference_record *lhs_rec = ir->lhs->as_dereference_record();
> + if (lhs_rec) {
> + ir_rvalue *lhs_rec_tmp = lhs_rec;
> + handle_rvalue(&lhs_rec_tmp);
> + if (lhs_rec_tmp != lhs_rec) {
> + ir->set_lhs(lhs_rec_tmp);
> + }
> + }
> + return rvalue_visit(ir);
> +}
> +
> +void
> +flatten_named_interface_blocks_declarations::handle_rvalue(ir_rvalue **rvalue)
> +{
> + if (*rvalue == NULL)
> + return;
> +
> + ir_dereference_record *ir = (*rvalue)->as_dereference_record();
> + if (ir == NULL)
> + return;
> +
> + ir_variable *var = ir->variable_referenced();
> +
> + if (!var->is_interface_instance())
> + return;
> +
> + /* It should be possible to handle uniforms during this pass,
> + * but, this will require changes to the other uniform block
> + * support code.
> + */
> + if (var->mode == ir_var_uniform)
> + return;
> +
> + if (var->interface_type != NULL) {
> + char *iface_field_name =
> + ralloc_asprintf(mem_ctx, "%s.%s", var->interface_type->name,
> + ir->field);
You might want to make a helper function for this, since both pieces of
code have to agree on the exact format of the hash table entry name.
> + /* Find the variable in the set of flattened interface blocks */
> + ir_variable *found_var =
> + (ir_variable *) hash_table_find(interface_namespace,
> + iface_field_name);
> + assert(found_var);
> + ir_dereference_variable *deref_var =
> + new(mem_ctx) ir_dereference_variable(found_var);
> + *rvalue = deref_var;
> + }
> +}
> +
> +void
> +lower_named_interface_blocks(void *mem_ctx, gl_shader *shader)
> +{
> + flatten_named_interface_blocks_declarations v_decl(mem_ctx);
> + v_decl.run(shader->ir);
> +}
> +
>
More information about the mesa-dev
mailing list