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

[Ian Romanick]

On 12/18/2016 10:09 PM, Timothy Arceri wrote:
> 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.

I believe you are correct.  I'll change that.

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