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

[Tapani Pälli]

On 08/18/2016 04:45 PM, Ilia Mirkin wrote:
> 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.

IMO this is fine, I originally had something along the lines of this but 
then I thought using hash_data would be more 'elegant way' to create 
unique values. I will check Eric's proposal also.

>   -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