Re: [Mesa-dev] [v2 5/6] glsl: functions to serialize gl_shader and gl_shader_program
On 11/06/2013 12:02 AM, Paul Berry wrote:
On 1 November 2013 02:16, Tapani Pälli tapani.pa...@intel.com
mailto:tapani.pa...@intel.com wrote:
+static void
+calc_item(const void *key, void *data, void *closure)
How about increment_count for the name of this function?
ok
+{
+ unsigned *sz = (unsigned *) closure;
+ *sz = *sz + 1;
+}
+
+
+static unsigned
+_hash_table_size(struct string_to_uint_map *map)
This function is global, so its name can't start with an
underscore--such names are reserved for library functions. Same goes
for many of the other functions in this file.
will change
+{
+ unsigned size = 0;
+ map-iterate(calc_item, size);
+ return size;
+}
+
+
+static void
+serialize_item(const void *key, void *data, void *closure)
+{
+ memory_writer *blob = (memory_writer *) closure;
+ uint32_t value = ((intptr_t)data);
+
+ blob-write_string((char *)key);
+ blob-write_uint32(value);
+}
+
+
+static void
+_serialize_hash_table(struct string_to_uint_map *map,
memory_writer *blob)
+{
+ uint32_t size = _hash_table_size(map);
+ blob-write_uint32(size);
+ map-iterate(serialize_item, blob);
Rather than take two passes over the hash table (one to compute its
size and one to output its contents), why not do the trick that we do
with overwrite() in ir_cache_serializer.cpp? (In other words, reserve
space for the size of the hashtable in bytes, then serialize the
hashtable, then overwrite the reserved space with the actual size).
yes this is faster, will change
+ /* Shaders IR, to be decided if we want these to be available */
Has there been previous discussion on the mesa-dev list about whether
or not we want these to be available? If so, please include a link to
previous discussion and a brief summary. if not, what does the
decision hinge on? At first blush it seems to me that there's no
reason to serialize the IR of the individual shaders, since
ARB_get_program_binary operates on full programs rather than
individual shaders.
Nope there has been no previous discussion about this. Automatic cache
will need these to be able to quick exit from glCompileShader (if
wanted), it's easier for the implementation to have them as separate
blobs than part of whole program blob though. With the extension I'm not
sure if unlinked IR is required for possible run-time recompilations?
I'd like someone to confirm this.
Also, typically we prefer to disable code using if (false) rather
than ifdefs, because that ensures that the disabled code still
compiles. (Otherwise, it's much more likely to bit rot as the rest of
the code base evolves).
ok, will use if (false), I also changethe unlinked shader define to use
your STORE_UNLINKED_SHADERS proposal
Are we going to attempt to store the back-end compiled shaders? It
seems like we'd have to do that in order to get a lot of the benefit
of ARB_get_program_binary.
Yes it is definitely required, there is big amount of time spent in
Driver-LinkShader(). However, this was left out for now. It might be
that several back-end blobs are needed and I'm not sure how well it
works and what should happen when recompilation happens. This needs a
bit more thought. At least new API is needed with driver to simply
retrieve and put binary blobs, I would certainly like some advice how to
approach the problem.
// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v2 5/6] glsl: functions to serialize gl_shader and gl_shader_program
On 6 November 2013 00:59, Tapani Pälli tapani.pa...@intel.com wrote:
On 11/06/2013 12:02 AM, Paul Berry wrote:
On 1 November 2013 02:16, Tapani Pälli tapani.pa...@intel.com wrote:
+static void
+calc_item(const void *key, void *data, void *closure)
How about increment_count for the name of this function?
ok
+{
+ unsigned *sz = (unsigned *) closure;
+ *sz = *sz + 1;
+}
+
+
+static unsigned
+_hash_table_size(struct string_to_uint_map *map)
This function is global, so its name can't start with an
underscore--such names are reserved for library functions. Same goes for
many of the other functions in this file.
will change
+{
+ unsigned size = 0;
+ map-iterate(calc_item, size);
+ return size;
+}
+
+
+static void
+serialize_item(const void *key, void *data, void *closure)
+{
+ memory_writer *blob = (memory_writer *) closure;
+ uint32_t value = ((intptr_t)data);
+
+ blob-write_string((char *)key);
+ blob-write_uint32(value);
+}
+
+
+static void
+_serialize_hash_table(struct string_to_uint_map *map, memory_writer
*blob)
+{
+ uint32_t size = _hash_table_size(map);
+ blob-write_uint32(size);
+ map-iterate(serialize_item, blob);
Rather than take two passes over the hash table (one to compute its size
and one to output its contents), why not do the trick that we do with
overwrite() in ir_cache_serializer.cpp? (In other words, reserve space for
the size of the hashtable in bytes, then serialize the hashtable, then
overwrite the reserved space with the actual size).
yes this is faster, will change
+ /* Shaders IR, to be decided if we want these to be available */
Has there been previous discussion on the mesa-dev list about whether or
not we want these to be available? If so, please include a link to
previous discussion and a brief summary. if not, what does the decision
hinge on? At first blush it seems to me that there's no reason to
serialize the IR of the individual shaders, since ARB_get_program_binary
operates on full programs rather than individual shaders.
Nope there has been no previous discussion about this. Automatic cache
will need these to be able to quick exit from glCompileShader (if wanted),
it's easier for the implementation to have them as separate blobs than part
of whole program blob though. With the extension I'm not sure if unlinked
IR is required for possible run-time recompilations? I'd like someone to
confirm this.
Yeah, you're probably right that we need to store the individual shaders'
IR for automatic caching.
For OES_get_program_binary, based on my reading of the spec, we shouldn't
store the individual shaders' IR. ProgramBinaryOES only loads the linked
shader; it leaves the compiled shader objects (if any) alone.
Also, typically we prefer to disable code using if (false) rather
than ifdefs, because that ensures that the disabled code still compiles.
(Otherwise, it's much more likely to bit rot as the rest of the code base
evolves).
ok, will use if (false), I also changethe unlinked shader define to use
your STORE_UNLINKED_SHADERS proposal
Are we going to attempt to store the back-end compiled shaders? It
seems like we'd have to do that in order to get a lot of the benefit of
ARB_get_program_binary.
Yes it is definitely required, there is big amount of time spent in
Driver-LinkShader(). However, this was left out for now. It might be that
several back-end blobs are needed and I'm not sure how well it works and
what should happen when recompilation happens. This needs a bit more
thought. At least new API is needed with driver to simply retrieve and put
binary blobs, I would certainly like some advice how to approach the
problem.
// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v2 5/6] glsl: functions to serialize gl_shader and gl_shader_program
On 1 November 2013 02:16, Tapani Pälli tapani.pa...@intel.com wrote:
These utility functions can be used to (de)serialize gl_shader and
gl_shader_program structures. This makes it possible to implement a
shader compiler cache for individual shaders and functionality required
by OES_get_program_binary extension.
Signed-off-by: Tapani Pälli tapani.pa...@intel.com
---
src/glsl/Makefile.sources | 1 +
src/glsl/ir_cache.cpp | 373
++
src/glsl/ir_cache.h | 58 +++
3 files changed, 432 insertions(+)
create mode 100644 src/glsl/ir_cache.cpp
create mode 100644 src/glsl/ir_cache.h
diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
index 81d5753..99b3c1a 100644
--- a/src/glsl/Makefile.sources
+++ b/src/glsl/Makefile.sources
@@ -30,6 +30,7 @@ LIBGLSL_FILES = \
$(GLSL_SRCDIR)/hir_field_selection.cpp \
$(GLSL_SRCDIR)/ir_basic_block.cpp \
$(GLSL_SRCDIR)/ir_builder.cpp \
+ $(GLSL_SRCDIR)/ir_cache.cpp \
$(GLSL_SRCDIR)/ir_cache_serializer.cpp \
$(GLSL_SRCDIR)/ir_cache_deserializer.cpp \
$(GLSL_SRCDIR)/ir_clone.cpp \
diff --git a/src/glsl/ir_cache.cpp b/src/glsl/ir_cache.cpp
new file mode 100644
index 000..24e1c77
--- /dev/null
+++ b/src/glsl/ir_cache.cpp
@@ -0,0 +1,373 @@
+/* -*- c++ -*- */
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
Software),
+ * to deal in the Software without restriction, including without
limitation
+ * the rights to use, copy, modify, merge, publish, distribute,
sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
next
+ * paragraph) shall be included in all copies or substantial portions of
the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND,
EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include main/shaderobj.h
+#include main/uniforms.h
+#include main/macros.h
+
+#include ir_cache_serializer.h
+#include ir_cache_deserializer.h
+
+/**
+ * Serialize gl_shader structure
+ */
+extern C char *
+_mesa_shader_serialize(struct gl_shader *shader,
+ struct _mesa_glsl_parse_state *state,
+ const char *mesa_sha, size_t *size)
+{
+ ir_serializer s;
+ return s.serialize(shader, state, mesa_sha, size);
+}
+
+
+static void
+calc_item(const void *key, void *data, void *closure)
How about increment_count for the name of this function?
+{
+ unsigned *sz = (unsigned *) closure;
+ *sz = *sz + 1;
+}
+
+
+static unsigned
+_hash_table_size(struct string_to_uint_map *map)
This function is global, so its name can't start with an underscore--such
names are reserved for library functions. Same goes for many of the other
functions in this file.
+{
+ unsigned size = 0;
+ map-iterate(calc_item, size);
+ return size;
+}
+
+
+static void
+serialize_item(const void *key, void *data, void *closure)
+{
+ memory_writer *blob = (memory_writer *) closure;
+ uint32_t value = ((intptr_t)data);
+
+ blob-write_string((char *)key);
+ blob-write_uint32(value);
+}
+
+
+static void
+_serialize_hash_table(struct string_to_uint_map *map, memory_writer *blob)
+{
+ uint32_t size = _hash_table_size(map);
+ blob-write_uint32(size);
+ map-iterate(serialize_item, blob);
Rather than take two passes over the hash table (one to compute its size
and one to output its contents), why not do the trick that we do with
overwrite() in ir_cache_serializer.cpp? (In other words, reserve space for
the size of the hashtable in bytes, then serialize the hashtable, then
overwrite the reserved space with the actual size).
+}
+
+
+static void
+_serialize_uniform_storage(gl_uniform_storage *uni, memory_writer blob)
+{
+ blob.write_string(uni-name);
+
+ save_glsl_type(blob, uni-type);
+
+ uint8_t initialized = uni-initialized;
+ uint8_t row_major = uni-row_major;
+
+ blob.write_uint32(uni-array_elements);
+ blob.write_uint8(initialized);
+ blob.write_int32(uni-block_index);
+ blob.write_int32(uni-offset);
+ blob.write_int32(uni-matrix_stride);
+ blob.write_uint8(row_major);
+ blob.write_int32(uni-atomic_buffer_index);
+
+ for (unsigned i = 0; i MESA_SHADER_TYPES; i++) {
+ uint8_t active =
[Mesa-dev] [v2 5/6] glsl: functions to serialize gl_shader and gl_shader_program
These utility functions can be used to (de)serialize gl_shader and
gl_shader_program structures. This makes it possible to implement a
shader compiler cache for individual shaders and functionality required
by OES_get_program_binary extension.
Signed-off-by: Tapani Pälli tapani.pa...@intel.com
---
src/glsl/Makefile.sources | 1 +
src/glsl/ir_cache.cpp | 373 ++
src/glsl/ir_cache.h | 58 +++
3 files changed, 432 insertions(+)
create mode 100644 src/glsl/ir_cache.cpp
create mode 100644 src/glsl/ir_cache.h
diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
index 81d5753..99b3c1a 100644
--- a/src/glsl/Makefile.sources
+++ b/src/glsl/Makefile.sources
@@ -30,6 +30,7 @@ LIBGLSL_FILES = \
$(GLSL_SRCDIR)/hir_field_selection.cpp \
$(GLSL_SRCDIR)/ir_basic_block.cpp \
$(GLSL_SRCDIR)/ir_builder.cpp \
+ $(GLSL_SRCDIR)/ir_cache.cpp \
$(GLSL_SRCDIR)/ir_cache_serializer.cpp \
$(GLSL_SRCDIR)/ir_cache_deserializer.cpp \
$(GLSL_SRCDIR)/ir_clone.cpp \
diff --git a/src/glsl/ir_cache.cpp b/src/glsl/ir_cache.cpp
new file mode 100644
index 000..24e1c77
--- /dev/null
+++ b/src/glsl/ir_cache.cpp
@@ -0,0 +1,373 @@
+/* -*- c++ -*- */
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include main/shaderobj.h
+#include main/uniforms.h
+#include main/macros.h
+
+#include ir_cache_serializer.h
+#include ir_cache_deserializer.h
+
+/**
+ * Serialize gl_shader structure
+ */
+extern C char *
+_mesa_shader_serialize(struct gl_shader *shader,
+ struct _mesa_glsl_parse_state *state,
+ const char *mesa_sha, size_t *size)
+{
+ ir_serializer s;
+ return s.serialize(shader, state, mesa_sha, size);
+}
+
+
+static void
+calc_item(const void *key, void *data, void *closure)
+{
+ unsigned *sz = (unsigned *) closure;
+ *sz = *sz + 1;
+}
+
+
+static unsigned
+_hash_table_size(struct string_to_uint_map *map)
+{
+ unsigned size = 0;
+ map-iterate(calc_item, size);
+ return size;
+}
+
+
+static void
+serialize_item(const void *key, void *data, void *closure)
+{
+ memory_writer *blob = (memory_writer *) closure;
+ uint32_t value = ((intptr_t)data);
+
+ blob-write_string((char *)key);
+ blob-write_uint32(value);
+}
+
+
+static void
+_serialize_hash_table(struct string_to_uint_map *map, memory_writer *blob)
+{
+ uint32_t size = _hash_table_size(map);
+ blob-write_uint32(size);
+ map-iterate(serialize_item, blob);
+}
+
+
+static void
+_serialize_uniform_storage(gl_uniform_storage *uni, memory_writer blob)
+{
+ blob.write_string(uni-name);
+
+ save_glsl_type(blob, uni-type);
+
+ uint8_t initialized = uni-initialized;
+ uint8_t row_major = uni-row_major;
+
+ blob.write_uint32(uni-array_elements);
+ blob.write_uint8(initialized);
+ blob.write_int32(uni-block_index);
+ blob.write_int32(uni-offset);
+ blob.write_int32(uni-matrix_stride);
+ blob.write_uint8(row_major);
+ blob.write_int32(uni-atomic_buffer_index);
+
+ for (unsigned i = 0; i MESA_SHADER_TYPES; i++) {
+ uint8_t active = uni-sampler[i].active;
+ blob.write_uint8(uni-sampler[i].index);
+ blob.write_uint8(active);
+ }
+
+ const unsigned elements = MAX2(1, uni-array_elements);
+ const unsigned data_components = elements * uni-type-components();
+ uint32_t size = elements * MAX2(1, data_components);
+
+ CACHE_DEBUG(%s: size %ld\n, __func__,
+ size * sizeof(union gl_constant_value));
+
+ blob.write_uint32(size);
+ blob.write(uni-storage, size * sizeof(union gl_constant_value));
+}
+
+
+/**
+ * Serialize gl_shader_program structure
+ */
+extern C char *
+_mesa_program_serialize(struct gl_shader_program *prog, size_t *size,
+ const char *mesa_sha)
+{
+ memory_writer blob;
+
+ blob.write_uint32(prog-Type);
+ blob.write_uint32(prog-NumShaders);
+
