<div dir="ltr">On 6 November 2013 00:59, Tapani Pälli <span dir="ltr"><<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><div class="im">
<div>On 11/06/2013 12:02 AM, Paul Berry
wrote:<br>
</div>
</div><blockquote type="cite">
<div dir="ltr"><div class="im">On 1 November 2013 02:16, Tapani Pälli <span dir="ltr"><<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>></span>
wrote:<br>
</div><div class="im"><div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+static
void<br>
+calc_item(const void *key, void *data, void *closure)<br>
</blockquote>
<div><br>
</div>
<div>How about "increment_count" for the name of this
function? <br>
</div>
</div>
</div>
</div></div>
</blockquote>
<br>
ok<div class="im"><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+{<br>
+ unsigned *sz = (unsigned *) closure;<br>
+ *sz = *sz + 1;<br>
+}<br>
+<br>
+<br>
+static unsigned<br>
+_hash_table_size(struct string_to_uint_map *map)<br>
</blockquote>
<div><br>
</div>
<div>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. <br>
</div>
</div>
</div>
</div>
</blockquote>
<br></div>
will change<div><div class="h5"><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+{<br>
+ unsigned size = 0;<br>
+ map->iterate(calc_item, &size);<br>
+ return size;<br>
+}<br>
+<br>
+<br>
+static void<br>
+serialize_item(const void *key, void *data, void
*closure)<br>
+{<br>
+ memory_writer *blob = (memory_writer *) closure;<br>
+ uint32_t value = ((intptr_t)data);<br>
+<br>
+ blob->write_string((char *)key);<br>
+ blob->write_uint32(&value);<br>
+}<br>
+<br>
+<br>
+static void<br>
+_serialize_hash_table(struct string_to_uint_map *map,
memory_writer *blob)<br>
+{<br>
+ uint32_t size = _hash_table_size(map);<br>
+ blob->write_uint32(&size);<br>
+ map->iterate(serialize_item, blob);<br>
</blockquote>
<div><br>
</div>
<div>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).<br>
</div>
<div> </div>
</div>
</div>
</div>
</blockquote>
<br></div></div>
yes this is faster, will change<div class="im"><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ /* Shaders IR, to be decided if we want these to be
available */<br>
</blockquote>
<div><br>
</div>
<div>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.<br>
<br>
</div>
</div>
</div>
</div>
</blockquote>
<br></div>
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.</div></blockquote><div><br></div><div>Yeah, you're probably right that we need to store the individual shaders' IR for automatic caching.<br><br>
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.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><div class="im"><br>
<br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div>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).<br>
</div>
</div>
</div>
</div>
</blockquote>
<br></div>
ok, will use if (false), I also changethe unlinked shader define to
use your STORE_UNLINKED_SHADERS proposal<div class="im"><br>
<br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div>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.<br>
</div>
<div>
</div>
</div>
</div>
</div>
</blockquote>
<br></div>
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.<span class=""><font color="#888888"><br>
<br>
// Tapani<br>
<br>
</font></span></div>
</blockquote></div><br></div></div>