[Mesa-dev] [PATCH] glsl: fix key used for hashing switch statement cases

[Ilia Mirkin]

Since it *is* a hash, you could just use a much simpler hash function like

int hash(int val) {
  int ret = val + 1000;
  if (ret == 0) ret = 1;
  return ret;
}

That way everything will map to a unique integer, except for -1000
which will map to the same thing as -999. I posit that this should be
rare, and hash collisions are perfectly fine in any case (it's not
like we have 1<<32 buckets anyways). This also avoids the zero hash
value case.

  -ilia


On Thu, Aug 18, 2016 at 7:52 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> Can the hash value not be 0?
>
>
> On Aug 18, 2016 4:57 AM, "Tapani Pälli" <tapani.palli at intel.com> wrote:
>>
>> Implementation previously used case value itself as the key, however
>> afterhash implementation change by ee02a5e we cannot use 0 as key.
>> Patch uses _mesa_hash_data to formulate a suitable key for this hash.
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97309
>> ---
>>  src/compiler/glsl/ast_to_hir.cpp | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/compiler/glsl/ast_to_hir.cpp
>> b/src/compiler/glsl/ast_to_hir.cpp
>> index e03a6e3..53fc4d6 100644
>> --- a/src/compiler/glsl/ast_to_hir.cpp
>> +++ b/src/compiler/glsl/ast_to_hir.cpp
>> @@ -6180,9 +6180,11 @@ ast_case_label::hir(exec_list *instructions,
>>           /* Stuff a dummy value in to allow processing to continue. */
>>           label_const = new(ctx) ir_constant(0);
>>        } else {
>> +         unsigned hash_key = _mesa_hash_data(&label_const->value.u[0],
>> +                                             sizeof(unsigned));
>>           ast_expression *previous_label = (ast_expression *)
>>           hash_table_find(state->switch_state.labels_ht,
>> -                         (void *)(uintptr_t)label_const->value.u[0]);
>> +                         (void *)(uintptr_t)hash_key);
>>
>>           if (previous_label) {
>>              YYLTYPE loc = this->test_value->get_location();
>> @@ -6193,7 +6195,7 @@ ast_case_label::hir(exec_list *instructions,
>>           } else {
>>              hash_table_insert(state->switch_state.labels_ht,
>>                                this->test_value,
>> -                              (void
>> *)(uintptr_t)label_const->value.u[0]);
>> +                              (void *)(uintptr_t)hash_key);
>>           }
>>        }
>>
>> --
>> 2.5.5
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev