Re: [Mesa-dev] [PATCH 04/19] glsl: Use simpler visitor to determine which UBO and SSBO blocks are used
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
>>
>> 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
>> Cc: mesa-sta...@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 000..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
___
mesa-dev mailing list
mes
Re: [Mesa-dev] [PATCH 04/19] glsl: Use simpler visitor to determine which UBO and SSBO blocks are used
On Thu, 2016-12-15 at 20:10 -0800, Ian Romanick wrote:
> From: Ian Romanick
>
> 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
> Cc: mesa-sta...@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 000..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
> + 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.
>
