[Mesa-dev] [PATCH 09/23] glsl: Convert constant_expression to the util hash table

[Thomas Helland]

2016-09-11 6:27 GMT+02:00 Timothy Arceri <timothy.arceri at collabora.com>:
> 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.com>:
>> >
>> > 2016-08-16 22:10 GMT+02:00 Thomas Helland <thomashelland90 at gmail.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_expression_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_pointer_c
>> > > ompare);
>> > > +   hash_table *deref_hash = _mesa_hash_table_create(NULL,
>> > > _mesa_hash_pointer,
>> > > +                                                    _mesa_key_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

Thanks for looking at this :)