[Mesa-stable] [Mesa-dev] [PATCH 04/19] glsl: Use simpler visitor to determine which UBO and SSBO blocks are used

Timothy Arceri timothy.arceri at collabora.com
Mon Dec 19 06:09:26 UTC 2016


On Thu, 2016-12-15 at 20:10 -0800, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> Very soon this visitor will get more complicated.  The users of the
> existing ir_variable_refcount visitor won't need the coming
> functionality, and this use doesn't need much of the functionality of
> ir_variable_refcount.
> 
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Cc: mesa-stable at lists.freedesktop.org
> ---
>  src/compiler/Makefile.sources           |   2 +
>  src/compiler/glsl/ir_array_refcount.cpp | 100
> ++++++++++++++++++++++++++++++++
>  src/compiler/glsl/ir_array_refcount.h   |  65 +++++++++++++++++++++
>  src/compiler/glsl/link_uniforms.cpp     |  10 ++--
>  4 files changed, 172 insertions(+), 5 deletions(-)
>  create mode 100644 src/compiler/glsl/ir_array_refcount.cpp
>  create mode 100644 src/compiler/glsl/ir_array_refcount.h
> 
> diff --git a/src/compiler/Makefile.sources
> b/src/compiler/Makefile.sources
> index 17b15de..15f410f 100644
> --- a/src/compiler/Makefile.sources
> +++ b/src/compiler/Makefile.sources
> @@ -29,6 +29,8 @@ LIBGLSL_FILES = \
>  	glsl/glsl_to_nir.cpp \
>  	glsl/glsl_to_nir.h \
>  	glsl/hir_field_selection.cpp \
> +	glsl/ir_array_refcount.cpp \
> +	glsl/ir_array_refcount.h \
>  	glsl/ir_basic_block.cpp \
>  	glsl/ir_basic_block.h \
>  	glsl/ir_builder.cpp \
> diff --git a/src/compiler/glsl/ir_array_refcount.cpp
> b/src/compiler/glsl/ir_array_refcount.cpp
> new file mode 100644
> index 0000000..41a0914
> --- /dev/null
> +++ b/src/compiler/glsl/ir_array_refcount.cpp
> @@ -0,0 +1,100 @@
> +/*
> + * Copyright © 2016 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 ir_array_refcount.cpp
> + *
> + * Provides a visitor which produces a list of variables referenced.
> + */
> +
> +#include "ir.h"
> +#include "ir_visitor.h"
> +#include "ir_array_refcount.h"
> +#include "compiler/glsl_types.h"
> +#include "util/hash_table.h"
> +
> +ir_array_refcount_visitor::ir_array_refcount_visitor()
> +{
> +   this->mem_ctx = ralloc_context(NULL);
> +   this->ht = _mesa_hash_table_create(NULL, _mesa_hash_pointer,
> +                                      _mesa_key_pointer_equal);
> +}
> +
> +static void
> +free_entry(struct hash_entry *entry)
> +{
> +   ir_array_refcount_entry *ivre = (ir_array_refcount_entry *)
> entry->data;
> +   delete ivre;
> +}
> +
> +ir_array_refcount_visitor::~ir_array_refcount_visitor()
> +{
> +   ralloc_free(this->mem_ctx);
> +   _mesa_hash_table_destroy(this->ht, free_entry);
> +}
> +
> +ir_array_refcount_entry::ir_array_refcount_entry(ir_variable *var)
> +   : var(var), is_referenced(false)
> +{
> +   /* empty */
> +}
> +
> +
> +ir_array_refcount_entry *
> +ir_array_refcount_visitor::get_variable_entry(ir_variable *var)
> +{
> +   assert(var);
> +
> +   struct hash_entry *e = _mesa_hash_table_search(this->ht, var);
> +   if (e)
> +      return (ir_array_refcount_entry *)e->data;
> +
> +   ir_array_refcount_entry *entry = new
> ir_array_refcount_entry(var);
> +   _mesa_hash_table_insert(this->ht, var, entry);
> +
> +   return entry;
> +}
> +
> +
> +ir_visitor_status
> +ir_array_refcount_visitor::visit(ir_dereference_variable *ir)
> +{
> +   ir_variable *const var = ir->variable_referenced();
> +   ir_array_refcount_entry *entry = this->get_variable_entry(var);
> +
> +   if (entry)

I don't think this can ever be not true. maybe just assert(entry). If
new fails it will throw an exception rather than return null as far as
I understand.

Otherwise seems fine:

Reviewed-by: Timothy Arceri <timothy.arceri at collabora.com>

> +      entry->is_referenced = true;
> +
> +   return visit_continue;
> +}
> +
> +
> +ir_visitor_status
> +ir_array_refcount_visitor::visit_enter(ir_function_signature *ir)
> +{
> +   /* We don't want to descend into the function parameters and
> +    * dead-code eliminate them, so just accept the body here.
> +    */
> +   visit_list_elements(this, &ir->body);
> +   return visit_continue_with_parent;
> +}
> diff --git a/src/compiler/glsl/ir_array_refcount.h
> b/src/compiler/glsl/ir_array_refcount.h
> new file mode 100644
> index 0000000..9ec46d2
> --- /dev/null
> +++ b/src/compiler/glsl/ir_array_refcount.h
> @@ -0,0 +1,65 @@
> +/*
> + * Copyright © 2016 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 ir_array_refcount.h
> + *
> + * Provides a visitor which produces a list of variables referenced.
> + */
> +
> +#include "ir.h"
> +#include "ir_visitor.h"
> +#include "compiler/glsl_types.h"
> +
> +class ir_array_refcount_entry
> +{
> +public:
> +   ir_array_refcount_entry(ir_variable *var);
> +
> +   ir_variable *var; /* The key: the variable's pointer. */
> +
> +   /** Has the variable been referenced? */
> +   bool is_referenced;
> +};
> +
> +class ir_array_refcount_visitor : public ir_hierarchical_visitor {
> +public:
> +   ir_array_refcount_visitor(void);
> +   ~ir_array_refcount_visitor(void);
> +
> +   virtual ir_visitor_status visit(ir_dereference_variable *);
> +
> +   virtual ir_visitor_status visit_enter(ir_function_signature *);
> +
> +   /**
> +    * Find variable in the hash table, and insert it if not present
> +    */
> +   ir_array_refcount_entry *get_variable_entry(ir_variable *var);
> +
> +   /**
> +    * Hash table mapping ir_variable to ir_array_refcount_entry.
> +    */
> +   struct hash_table *ht;
> +
> +   void *mem_ctx;
> +};
> diff --git a/src/compiler/glsl/link_uniforms.cpp
> b/src/compiler/glsl/link_uniforms.cpp
> index dd6336e..2f851ce 100644
> --- a/src/compiler/glsl/link_uniforms.cpp
> +++ b/src/compiler/glsl/link_uniforms.cpp
> @@ -28,7 +28,7 @@
>  #include "glsl_symbol_table.h"
>  #include "program.h"
>  #include "util/string_to_uint_map.h"
> -#include "ir_variable_refcount.h"
> +#include "ir_array_refcount.h"
>  
>  /**
>   * \file link_uniforms.cpp
> @@ -868,11 +868,11 @@ public:
>  };
>  
>  static bool
> -variable_is_referenced(ir_variable_refcount_visitor &v, ir_variable
> *var)
> +variable_is_referenced(ir_array_refcount_visitor &v, ir_variable
> *var)
>  {
> -   ir_variable_refcount_entry *const entry =
> v.get_variable_entry(var);
> +   ir_array_refcount_entry *const entry = v.get_variable_entry(var);
>  
> -   return entry->referenced_count > 0;
> +   return entry->is_referenced;
>  
>  }
>  
> @@ -886,7 +886,7 @@ static void
>  link_update_uniform_buffer_variables(struct gl_linked_shader
> *shader,
>                                       unsigned stage)
>  {
> -   ir_variable_refcount_visitor v;
> +   ir_array_refcount_visitor v;
>  
>     v.run(shader->ir);
>  


More information about the mesa-stable mailing list