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
