[Mesa-dev] [PATCH 09/23] glsl: Convert constant_expression to the util hash table
Timothy Arceri
timothy.arceri at collabora.com
Mon Sep 12 00:58:58 UTC 2016
On Sun, 2016-09-11 at 15:31 +0200, Thomas Helland wrote:
> 2016-09-11 6:27 GMT+02:00 Timothy Arceri <timothy.arceri at collabora.co
> m>:
> >
> > On Sat, 2016-09-10 at 13:19 +0200, Thomas Helland wrote:
> > >
> > > 2016-09-09 0:20 GMT+02:00 Thomas Helland <thomashelland90 at gmail.c
> > > om>:
> > > >
> > > >
> > > > 2016-08-16 22:10 GMT+02:00 Thomas Helland <thomashelland90 at gmai
> > > > l.co
> > > > m>:
> > > > >
> > > > >
> > > > > Signed-off-by: Thomas Helland <thomashelland90 at gmail.com>
> > > > > ---
> > > > > src/compiler/glsl/ir_constant_expression.cpp | 24
> > > > > +++++++++++++-
> > > > > ----------
> > > > > 1 file changed, 13 insertions(+), 11 deletions(-)
> > > > >
> > > > > diff --git a/src/compiler/glsl/ir_constant_expression.cpp
> > > > > b/src/compiler/glsl/ir_constant_expression.cpp
> > > > > index 6329acd..16c8fac 100644
> > > > > --- a/src/compiler/glsl/ir_constant_expression.cpp
> > > > > +++ b/src/compiler/glsl/ir_constant_expression.cpp
> > > > > @@ -39,7 +39,7 @@
> > > > > #include "util/half_float.h"
> > > > > #include "ir.h"
> > > > > #include "compiler/glsl_types.h"
> > > > > -#include "program/hash_table.h"
> > > > > +#include "util/hash_table.h"
> > > > >
> > > > > static float
> > > > > dot_f(ir_constant *op0, ir_constant *op1)
> > > > > @@ -457,7 +457,8 @@ constant_referenced(const ir_dereference
> > > > > *deref,
> > > > > const ir_dereference_variable *const dv =
> > > > > (const ir_dereference_variable *) deref;
> > > > >
> > > > > - store = (ir_constant *)
> > > > > hash_table_find(variable_context,
> > > > > dv->var);
> > > > > + hash_entry *entry =
> > > > > _mesa_hash_table_search(variable_context, dv->var);
> > > > > + store = (ir_constant *) entry->data;
> > > > > break;
> > > > > }
> > > > >
> > > > > @@ -1806,9 +1807,10 @@
> > > > > ir_dereference_variable::constant_expression_value(struct
> > > > > hash_table *variable_c
> > > > >
> > > > > /* Give priority to the context hashtable, if it exists
> > > > > */
> > > > > if (variable_context) {
> > > > > - ir_constant *value = (ir_constant
> > > > > *)hash_table_find(variable_context, var);
> > > > > - if(value)
> > > > > - return value;
> > > > > + hash_entry *entry =
> > > > > _mesa_hash_table_search(variable_context, var);
> > > > > +
> > > > > + if(entry)
> > > > > + return (ir_constant *) entry->data;
> > > > > }
> > > > >
> > > > > /* The constant_value of a uniform variable is its
> > > > > initializer,
> > > > > @@ -1926,7 +1928,7 @@ bool
> > > > > ir_function_signature::constant_expression_evaluate_expressio
> > > > > n_li
> > > > > st(const s
> > > > > /* (declare () type symbol) */
> > > > > case ir_type_variable: {
> > > > > ir_variable *var = inst->as_variable();
> > > > > - hash_table_insert(variable_context,
> > > > > ir_constant::zero(this, var->type), var);
> > > > > + _mesa_hash_table_insert(variable_context, var,
> > > > > ir_constant::zero(this, var->type));
> > > > > break;
> > > > > }
> > > > >
> > > > > @@ -2050,8 +2052,8 @@
> > > > > ir_function_signature::constant_expression_value(exec_list
> > > > > *actual_parameters, s
> > > > > * We expect the correctness of the number of parameters
> > > > > to
> > > > > have
> > > > > * been checked earlier.
> > > > > */
> > > > > - hash_table *deref_hash = hash_table_ctor(8,
> > > > > hash_table_pointer_hash,
> > > > > - hash_table_point
> > > > > er_c
> > > > > ompare);
> > > > > + hash_table *deref_hash = _mesa_hash_table_create(NULL,
> > > > > _mesa_hash_pointer,
> > > > > + _mesa_ke
> > > > > y_po
> > > > > inter_equal);
> > > > >
> > > > > /* If "origin" is non-NULL, then the function body is
> > > > > there. So we
> > > > > * have to use the variable objects from the object with
> > > > > the
> > > > > body,
> > > > > @@ -2062,13 +2064,13 @@
> > > > > ir_function_signature::constant_expression_value(exec_list
> > > > > *actual_parameters, s
> > > > > foreach_in_list(ir_rvalue, n, actual_parameters) {
> > > > > ir_constant *constant = n-
> > > > > >
> > > > > > constant_expression_value(variable_context);
> > > > > if (constant == NULL) {
> > > > > - hash_table_dtor(deref_hash);
> > > > > + _mesa_hash_table_destroy(deref_hash, NULL);
> > > > > return NULL;
> > > > > }
> > > > >
> > > > >
> > > > > ir_variable *var = (ir_variable *)parameter_info;
> > > > > - hash_table_insert(deref_hash, constant, var);
> > > > > + _mesa_hash_table_insert(deref_hash, constant, var);
> > > >
> > > > This would be the cause of the regressions.
> > > > The API is inverted between the hash table implementations,
> > > > but the arguments here are not. No wonder weird things happen.
> > > > Will do a complete piglit run (except deqp, etc) and send
> > > > an updated patch to the list likely sometime tomorrow.
> > > >
> > >
> > > So, I did a complete piglit run, and the only changes after
> > > fixing
> > > this issue are some likely spurious changes in some timing tests.
> > > Some gl_arb_sync_control tests goes from warn to pass, and some
> > > goes
> > > the opposite way, from pass to warn. ext-timer-query time-elapsed
> > > goes
> > > from pass to fail. I don't see how these are related to this
> > > change
> > > though.
> >
> > Yeah don't worry about those. Do you have a branch somewhere that I
> > can
> > grab and push to intels ci system?
> >
>
> Just pushed a branch to https://github.com/thohel/mesa/tree/prog-
> hash-rework
There were no regressions in the other test suites so I made a few
minor fixs/tidy ups, squashed the set patchs and pushed the series.
>
> Thanks for looking at this :)
Thanks for working on it. It's good to see this go away :)
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list