[Mesa-dev] [PATCH 28/40] glsl/mesa: make a copy of attribute bindings in case of cache fallback

[Nicolai Hähnle]

On 10.02.2017 13:13, Timothy Arceri wrote:
>
>
> On 10/02/17 22:25, Nicolai Hähnle wrote:
>> On 07.02.2017 04:42, Timothy Arceri wrote:
>>> From: Timothy Arceri <timothy.arceri at collabora.com>
>>>
>>> If the shader cache falls back to doing a compile and link we need the
>>> original attribute bindings as they could have changed after the program
>>> was first linked.
>>
>> So, same question as for patch #22, I don't really understand why the
>> fallback attributes should be necessary and what the data flow is,
>> since the cache_fallback happens during glLinkProgram().
>
> See reply to 22.
>
>>
>> On top of that:
>>
>>> ---
>>>  src/compiler/glsl/shader_cache.cpp | 15 +++++++++++++++
>>>  src/mesa/main/mtypes.h             |  7 +++++++
>>>  src/mesa/main/shaderobj.c          |  6 ++++++
>>>  3 files changed, 28 insertions(+)
>>>
>>> diff --git a/src/compiler/glsl/shader_cache.cpp
>>> b/src/compiler/glsl/shader_cache.cpp
>>> index 729dd09..ddcd530 100644
>>> --- a/src/compiler/glsl/shader_cache.cpp
>>> +++ b/src/compiler/glsl/shader_cache.cpp
>>> @@ -808,9 +808,22 @@ read_hash_table(struct blob_reader *metadata,
>>> struct string_to_uint_map *hash)
>>>  }
>>>
>>>  static void
>>> +copy_hash_entry(const void *key, void *data, void *closure)
>>> +{
>>> +   struct string_to_uint_map *ht = (struct string_to_uint_map *)
>>> closure;
>>> +
>>> +   /* string_to_uint_map increases everything by 1 so we need to
>>> allow for
>>> +    * this when copying the data directly.
>>> +    */
>>> +   ht->put(((intptr_t) data) - 1, (const char *) key);
>>> +}
>>> +
>>> +static void
>>>  write_hash_tables(struct blob *metadata, struct gl_shader_program
>>> *prog)
>>>  {
>>>     write_hash_table(metadata, prog->AttributeBindings);
>>> +   hash_table_call_foreach(prog->AttributeBindings->ht,
>>> copy_hash_entry,
>>> + prog->data->FallbackAttributeBindings);
>>>     write_hash_table(metadata, prog->FragDataBindings);
>>>     write_hash_table(metadata, prog->FragDataIndexBindings);
>>>  }
>>> @@ -819,6 +832,8 @@ static void
>>>  read_hash_tables(struct blob_reader *metadata, struct
>>> gl_shader_program *prog)
>>>  {
>>>     read_hash_table(metadata, prog->AttributeBindings);
>>> +   hash_table_call_foreach(prog->AttributeBindings->ht,
>>> copy_hash_entry,
>>> + prog->data->FallbackAttributeBindings);
>>
>> It seems rather suspicious that both write_hash_tables and
>> read_hash_tables perform the copy in the same direction, and I don't
>> see FallbackAttributeBindings being used anywhere else.
>
> I'm not sure what you mean about the direction (need to take a better
> look tomorrow its late here). As you point out tgsi fallback is done
> during linking so it doesn't need it. We use is for the fallback in i965
> as we have no NIR level cache [1].
>
> [1]
> https://github.com/tarceri/Mesa/commit/31089f44c9a74ab303d36616f4e9520bd5a62d05

That explains it, thanks!

Cheers,
Nicolai