[Mesa-dev] [PATCH 149/133] nir: Use the actual FNV-1a hash for hashing derefs
Connor Abbott
cwabbott0 at gmail.com
Fri Jan 9 10:40:24 PST 2015
Whoops... I was looking over the actual file (instead of the patch),
and there's a comment above (at least in lower_variables) about magic
numbers that needs to be deleted or rewritten.
On Fri, Jan 9, 2015 at 1:30 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
> As you mentioned, you should add something to the commit message
> saying you're making it use a loop too.
>
> On Tue, Jan 6, 2015 at 7:34 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>> ---
>> src/glsl/nir/nir_lower_locals_to_regs.c | 65 ++++++++------------
>> src/glsl/nir/nir_lower_variables.c | 101 +++++++++++++++++---------------
>> 2 files changed, 79 insertions(+), 87 deletions(-)
>>
>> diff --git a/src/glsl/nir/nir_lower_locals_to_regs.c b/src/glsl/nir/nir_lower_locals_to_regs.c
>> index fdbd4ae..d55618a 100644
>> --- a/src/glsl/nir/nir_lower_locals_to_regs.c
>> +++ b/src/glsl/nir/nir_lower_locals_to_regs.c
>> @@ -43,61 +43,48 @@ struct locals_to_regs_state {
>> static uint32_t
>> hash_deref(const void *void_deref)
>> {
>> - const nir_deref *deref = void_deref;
>> + uint32_t hash = _mesa_FNV32_1a_offset_bias;
>>
>> - uint32_t hash;
>> - if (deref->child) {
>> - hash = hash_deref(deref->child);
>> - } else {
>> - hash = 2166136261ul;
>> - }
>> + const nir_deref_var *deref_var = void_deref;
>> + hash = _mesa_FNV32_1a_accumulate(hash, deref_var->var);
>>
>> - switch (deref->deref_type) {
>> - case nir_deref_type_var:
>> - hash ^= _mesa_hash_pointer(nir_deref_as_var(deref)->var);
>> - break;
>> - case nir_deref_type_array: {
>> - hash ^= 268435183;
>> - break;
>> - }
>> - case nir_deref_type_struct:
>> - hash ^= nir_deref_as_struct(deref)->index;
>> - break;
>> + for (const nir_deref *deref = deref_var->deref.child;
>> + deref; deref = deref->child) {
>> + if (deref->deref_type == nir_deref_type_struct) {
>> + const nir_deref_struct *deref_struct = nir_deref_as_struct(deref);
>> + hash = _mesa_FNV32_1a_accumulate(hash, deref_struct->index);
>> + }
>> }
>>
>> - return hash * 0x01000193;
>> + return hash;
>> }
>>
>> static bool
>> derefs_equal(const void *void_a, const void *void_b)
>> {
>> - const nir_deref *a = void_a;
>> - const nir_deref *b = void_b;
>> + const nir_deref_var *a_var = void_a;
>> + const nir_deref_var *b_var = void_b;
>>
>> - if (a->deref_type != b->deref_type)
>> + if (a_var->var != b_var->var)
>> return false;
>>
>> - switch (a->deref_type) {
>> - case nir_deref_type_var:
>> - if (nir_deref_as_var(a)->var != nir_deref_as_var(b)->var)
>> + for (const nir_deref *a = a_var->deref.child, *b = b_var->deref.child;
>> + a != NULL; a = a->child, b = b->child) {
>> + if (a->deref_type != b->deref_type)
>> return false;
>> - break;
>> - case nir_deref_type_array:
>> - /* Do nothing. All array derefs are the same */
>> - break;
>> - case nir_deref_type_struct:
>> - if (nir_deref_as_struct(a)->index != nir_deref_as_struct(b)->index)
>> +
>> + if (a->deref_type == nir_deref_type_struct) {
>> + if (nir_deref_as_struct(a)->index != nir_deref_as_struct(b)->index)
>> + return false;
>> + }
>> + /* Do nothing for arrays. They're all the same. */
>> +
>> + assert((a->child == NULL) == (b->child == NULL));
>> + if((a->child == NULL) != (b->child == NULL))
>> return false;
>> - break;
>> - default:
>> - unreachable("Invalid dreference type");
>> }
>>
>> - assert((a->child == NULL) == (b->child == NULL));
>> - if (a->child)
>> - return derefs_equal(a->child, b->child);
>> - else
>> - return true;
>> + return true;
>> }
>>
>> static nir_register *
>> diff --git a/src/glsl/nir/nir_lower_variables.c b/src/glsl/nir/nir_lower_variables.c
>> index cf9818b..84cf85f 100644
>> --- a/src/glsl/nir/nir_lower_variables.c
>> +++ b/src/glsl/nir/nir_lower_variables.c
>> @@ -75,73 +75,78 @@ struct lower_variables_state {
>> static uint32_t
>> hash_deref(const void *void_deref)
>> {
>> - const nir_deref *deref = void_deref;
>> + uint32_t hash = _mesa_FNV32_1a_offset_bias;
>>
>> - uint32_t hash;
>> - if (deref->child) {
>> - hash = hash_deref(deref->child);
>> - } else {
>> - hash = 2166136261ul;
>> - }
>> + const nir_deref_var *deref_var = void_deref;
>> + hash = _mesa_FNV32_1a_accumulate(hash, deref_var->var);
>>
>> - switch (deref->deref_type) {
>> - case nir_deref_type_var:
>> - hash ^= _mesa_hash_pointer(nir_deref_as_var(deref)->var);
>> - break;
>> - case nir_deref_type_array: {
>> - nir_deref_array *array = nir_deref_as_array(deref);
>> - hash += 268435183 * array->deref_array_type;
>> - if (array->deref_array_type == nir_deref_array_type_direct)
>> - hash ^= array->base_offset; /* Some prime */
>> - break;
>> - }
>> - case nir_deref_type_struct:
>> - hash ^= nir_deref_as_struct(deref)->index;
>> - break;
>> + for (const nir_deref *deref = deref_var->deref.child;
>> + deref; deref = deref->child) {
>> + switch (deref->deref_type) {
>> + case nir_deref_type_array: {
>> + nir_deref_array *deref_array = nir_deref_as_array(deref);
>> +
>> + hash = _mesa_FNV32_1a_accumulate(hash, deref_array->deref_array_type);
>> +
>> + if (deref_array->deref_array_type == nir_deref_array_type_direct)
>> + hash = _mesa_FNV32_1a_accumulate(hash, deref_array->base_offset);
>> + break;
>> + }
>> + case nir_deref_type_struct: {
>> + nir_deref_struct *deref_struct = nir_deref_as_struct(deref);
>> + hash = _mesa_FNV32_1a_accumulate(hash, deref_struct->index);
>> + break;
>> + }
>> + default:
>> + assert("Invalid deref chain");
>> + }
>> }
>>
>> - return hash * 0x01000193;
>> + return hash;
>> }
>>
>> static bool
>> derefs_equal(const void *void_a, const void *void_b)
>> {
>> - const nir_deref *a = void_a;
>> - const nir_deref *b = void_b;
>> + const nir_deref_var *a_var = void_a;
>> + const nir_deref_var *b_var = void_b;
>>
>> - if (a->deref_type != b->deref_type)
>> + if (a_var->var != b_var->var)
>> return false;
>>
>> - switch (a->deref_type) {
>> - case nir_deref_type_var:
>> - if (nir_deref_as_var(a)->var != nir_deref_as_var(b)->var)
>> + for (const nir_deref *a = a_var->deref.child, *b = b_var->deref.child;
>> + a != NULL; a = a->child, b = b->child) {
>> + if (a->deref_type != b->deref_type)
>> return false;
>> - break;
>> - case nir_deref_type_array: {
>> - nir_deref_array *a_arr = nir_deref_as_array(a);
>> - nir_deref_array *b_arr = nir_deref_as_array(b);
>>
>> - if (a_arr->deref_array_type != b_arr->deref_array_type)
>> - return false;
>> + switch (a->deref_type) {
>> + case nir_deref_type_array: {
>> + nir_deref_array *a_arr = nir_deref_as_array(a);
>> + nir_deref_array *b_arr = nir_deref_as_array(b);
>>
>> - if (a_arr->deref_array_type == nir_deref_array_type_direct &&
>> - a_arr->base_offset != b_arr->base_offset)
>> + if (a_arr->deref_array_type != b_arr->deref_array_type)
>> + return false;
>> +
>> + if (a_arr->deref_array_type == nir_deref_array_type_direct &&
>> + a_arr->base_offset != b_arr->base_offset)
>> + return false;
>> + break;
>> + }
>> + case nir_deref_type_struct:
>> + if (nir_deref_as_struct(a)->index != nir_deref_as_struct(b)->index)
>> + return false;
>> + break;
>> + default:
>> + assert("Invalid deref chain");
>> return false;
>> - break;
>> - }
>> - case nir_deref_type_struct:
>> - if (nir_deref_as_struct(a)->index != nir_deref_as_struct(b)->index)
>> + }
>> +
>> + assert((a->child == NULL) == (b->child == NULL));
>> + if((a->child == NULL) != (b->child == NULL))
>> return false;
>
> Hmm, if you're already doing the assert I don't think the if is really
> necessary?
>
>> - break;
>> - default:
>> - unreachable("Invalid dreference type");
>> }
>>
>> - assert((a->child == NULL) == (b->child == NULL));
>> - if (a->child)
>> - return derefs_equal(a->child, b->child);
>> - else
>> - return true;
>> + return true;
>> }
>>
>> static int
>> --
>> 2.2.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> Other than the a few minor things,
>
> Reviewed-by: Connor Abbott <cwabbott0 at gmail.com>
More information about the mesa-dev
mailing list