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 <[email protected]> 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 <[email protected]> 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 >> [email protected] >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev > > Other than the a few minor things, > > Reviewed-by: Connor Abbott <[email protected]> _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
