On 10/02/17 22:25, Nicolai Hähnle wrote:
On 07.02.2017 04:42, Timothy Arceri wrote:
From: Timothy Arceri <timothy.arc...@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


Cheers,
Nicolai

    read_hash_table(metadata, prog->FragDataBindings);
    read_hash_table(metadata, prog->FragDataIndexBindings);
 }
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index f65cd76..d1dde0c 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2688,6 +2688,13 @@ struct gl_shader_program_data
    GLuint NumFallbackShaders;
struct gl_shader **FallbackShaders; /**< Shaders used for cache fallback */

+   /**
+ * 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.
+    */
+   struct string_to_uint_map *FallbackAttributeBindings;
+
    /** List of all active resources after linking. */
    struct gl_program_resource *ProgramResourceList;
    unsigned NumProgramResourceList;
diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
index ed19a72..245a0b9 100644
--- a/src/mesa/main/shaderobj.c
+++ b/src/mesa/main/shaderobj.c
@@ -276,6 +276,7 @@ init_shader_program(struct gl_shader_program *prog)
    prog->RefCount = 1;

    prog->AttributeBindings = string_to_uint_map_ctor();
+   prog->data->FallbackAttributeBindings = string_to_uint_map_ctor();
    prog->FragDataBindings = string_to_uint_map_ctor();
    prog->FragDataIndexBindings = string_to_uint_map_ctor();

@@ -393,6 +394,11 @@ _mesa_free_shader_program_data(struct gl_context *ctx,
       shProg->AttributeBindings = NULL;
    }

+   if (shProg->data->FallbackAttributeBindings) {
+ string_to_uint_map_dtor(shProg->data->FallbackAttributeBindings);
+      shProg->data->FallbackAttributeBindings = NULL;
+   }
+
    if (shProg->FragDataBindings) {
       string_to_uint_map_dtor(shProg->FragDataBindings);
       shProg->FragDataBindings = NULL;


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to