[Mesa-dev] [PATCH 08/19] glsl: Walk a list of ir_dereference_array to mark array elements as accessed

Kenneth Graunke kenneth at whitecape.org
Mon Dec 19 20:30:23 UTC 2016


On Thursday, December 15, 2016 8:10:20 PM PST Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Cc: mesa-stable at lists.freedesktop.org
> ---
>  src/compiler/glsl/ir_array_refcount.cpp         |  73 +++-
>  src/compiler/glsl/ir_array_refcount.h           |  10 +
>  src/compiler/glsl/tests/array_refcount_test.cpp | 425 ++++++++++++++++++++++++
>  3 files changed, 507 insertions(+), 1 deletion(-)
> 
> diff --git a/src/compiler/glsl/ir_array_refcount.cpp b/src/compiler/glsl/ir_array_refcount.cpp
> index 75dca0a..7c9ab8e 100644
> --- a/src/compiler/glsl/ir_array_refcount.cpp
> +++ b/src/compiler/glsl/ir_array_refcount.cpp
> @@ -34,7 +34,7 @@
>  #include "util/hash_table.h"
>  
>  ir_array_refcount_visitor::ir_array_refcount_visitor()
> -   : derefs(0), num_derefs(0), derefs_size(0)
> +   : last_array_deref(0), derefs(0), num_derefs(0), derefs_size(0)
>  {
>     this->mem_ctx = ralloc_context(NULL);
>     this->ht = _mesa_hash_table_create(NULL, _mesa_hash_pointer,
> @@ -159,6 +159,77 @@ ir_array_refcount_visitor::get_array_deref()
>     return d;
>  }
>  
> +ir_visitor_status
> +ir_array_refcount_visitor::visit_enter(ir_dereference_array *ir)
> +{
> +   /* It could also be a vector or a matrix.  Individual elements of vectors
> +    * are natrices are not tracked, so bail.

         and matrices 

> +    */
> +   if (!ir->array->type->is_array())
> +      return visit_continue;
> +
> +   /* If this array dereference is a child of an array dereference that was
> +    * already visited, just continue on.  Otherwise, for an arrays-of-arrays
> +    * dereference like x[1][2][3][4], we'd process the [1][2][3][4] sequence,
> +    * the [1][2][3] sequence, the [1][2] sequence, and the [1] sequence.  This
> +    * ensures that we only process the full sequence.
> +    */
> +   if (last_array_deref && last_array_deref->array == ir) {
> +      last_array_deref = ir;
> +      return visit_continue;
> +   }
> +
> +   last_array_deref = ir;
> +
> +   num_derefs = 0;
> +
> +   ir_rvalue *rv = ir;
> +   while (rv->ir_type == ir_type_dereference_array) {
> +      ir_dereference_array *const deref = rv->as_dereference_array();
> +
> +      assert(deref != NULL);
> +      assert(deref->array->type->is_array());
> +
> +      ir_rvalue *const array = deref->array;
> +      const ir_constant *const idx = deref->array_index->as_constant();
> +      array_deref_range *const dr = get_array_deref();
> +
> +      dr->size = array->type->array_size();
> +
> +      if (idx != NULL) {
> +         dr->index = idx->get_int_component(0);
> +      } else {
> +         /* An unsized array can occur at the end of an SSBO.  We can't track
> +          * accesses to such an array, so bail.
> +          */
> +         if (array->type->array_size() == 0)
> +            return visit_continue;
> +
> +         dr->index = dr->size;
> +      }
> +
> +      rv = array;
> +   }
> +
> +   ir_dereference_variable *const var_deref = rv->as_dereference_variable();
> +
> +   /* If the array being dereferenced is not a variable, bail.  At the very
> +    * least, ir_constant and ir_dereference_record are possible.
> +    */
> +   if (var_deref == NULL)
> +      return visit_continue;
> +
> +   ir_array_refcount_entry *const entry =
> +      this->get_variable_entry(var_deref->var);
> +
> +   if (entry == NULL)
> +      return visit_stop;
> +
> +   entry->mark_array_elements_referenced(derefs, num_derefs);
> +
> +   return visit_continue;
> +}
> +
>  
>  ir_visitor_status
>  ir_array_refcount_visitor::visit(ir_dereference_variable *ir)
> diff --git a/src/compiler/glsl/ir_array_refcount.h b/src/compiler/glsl/ir_array_refcount.h
> index 2988046..46ba36c 100644
> --- a/src/compiler/glsl/ir_array_refcount.h
> +++ b/src/compiler/glsl/ir_array_refcount.h
> @@ -140,6 +140,7 @@ public:
>     virtual ir_visitor_status visit(ir_dereference_variable *);
>  
>     virtual ir_visitor_status visit_enter(ir_function_signature *);
> +   virtual ir_visitor_status visit_enter(ir_dereference_array *);
>  
>     /**
>      * Find variable in the hash table, and insert it if not present
> @@ -158,6 +159,15 @@ private:
>     array_deref_range *get_array_deref();
>  
>     /**
> +    * Last ir_dereference_array that was visited
> +    *
> +    * Used to prevent some redundant calculations.
> +    *
> +    * \sa ::visit_enter(ir_dereference_array *)
> +    */
> +   ir_dereference_array *last_array_deref;
> +
> +   /**
>      * \name array_deref_range tracking
>      */
>     /*@{*/
> diff --git a/src/compiler/glsl/tests/array_refcount_test.cpp b/src/compiler/glsl/tests/array_refcount_test.cpp
> index 29bb133..ecd7f46 100644
> --- a/src/compiler/glsl/tests/array_refcount_test.cpp
> +++ b/src/compiler/glsl/tests/array_refcount_test.cpp
> @@ -23,12 +23,18 @@
>  #include <gtest/gtest.h>
>  #include "ir.h"
>  #include "ir_array_refcount.h"
> +#include "ir_builder.h"
> +#include "util/hash_table.h"
> +
> +using namespace ir_builder;
>  
>  class array_refcount_test : public ::testing::Test {
>  public:
>     virtual void SetUp();
>     virtual void TearDown();
>  
> +   exec_list instructions;
> +   ir_factory *body;
>     void *mem_ctx;
>  
>     /**
> @@ -40,6 +46,14 @@ public:
>     const glsl_type *array_3_of_array_4_of_array_5_of_vec4;
>  
>     /**
> +    * glsl_type for a int[3].
> +    *
> +    * The exceptionally verbose name is picked because it matches the syntax
> +    * of http://cdecl.org/.
> +    */
> +   const glsl_type *array_3_of_int;
> +
> +   /**
>      * Wrapper to access private member "bits" of ir_array_refcount_entry
>      *
>      * The test class is a friend to ir_array_refcount_entry, but the
> @@ -81,6 +95,9 @@ array_refcount_test::SetUp()
>  {
>     mem_ctx = ralloc_context(NULL);
>  
> +   instructions.make_empty();
> +   body = new ir_factory(&instructions, mem_ctx);
> +
>     /* The type of vec4 x[3][4][5]; */
>     const glsl_type *const array_5_of_vec4 =
>        glsl_type::get_array_instance(glsl_type::vec4_type, 5);
> @@ -88,15 +105,105 @@ array_refcount_test::SetUp()
>        glsl_type::get_array_instance(array_5_of_vec4, 4);
>     array_3_of_array_4_of_array_5_of_vec4 =
>        glsl_type::get_array_instance(array_4_of_array_5_of_vec4, 3);
> +
> +   array_3_of_int = glsl_type::get_array_instance(glsl_type::int_type, 3);
>  }
>  
>  void
>  array_refcount_test::TearDown()
>  {
> +   delete body;
> +   body = NULL;
> +
>     ralloc_free(mem_ctx);
>     mem_ctx = NULL;
>  }
>  
> +static operand
> +deref_array(operand array, operand index)

I'd suggest just making this return the ir_rvalue *, rather than
wrapping it in an operand.  It's not necessary - if you take the
resulting ir_rvalue *, and pass it to anything expecting an operand,
it'll automatically create one for you.  But you can also just use it
as a normal piece of IR that way.

> +{
> +   void *mem_ctx = ralloc_parent(array.val);
> +
> +   ir_rvalue *val = new(mem_ctx) ir_dereference_array(array.val, index.val);
> +
> +   return operand(val);
> +}
> +
> +static operand
> +deref_struct(operand s, const char *field)

Ditto.

It might also make sense to just put these in ir_builder, as well.

Either way, the series is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161219/210b1284/attachment.sig>


More information about the mesa-dev mailing list