<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 01/13/2014 11:58 PM, Paul Berry
wrote:<br>
</div>
<blockquote
cite="mid:CA+yLL66G3rZ19_PKKh0GaudfOH_bAzBJAEHoy+J2qHam3FxOrQ@mail.gmail.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=ISO-8859-1">
<div dir="ltr">On 2 January 2014 03:58, Tapani Pälli <span
dir="ltr"><<a moz-do-not-send="true"
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">diff --git
a/src/glsl/ir_serialize.cpp b/src/glsl/ir_serialize.cpp<br>
new file mode 100644<br>
index 0000000..30ca018<br>
--- /dev/null<br>
+++ b/src/glsl/ir_serialize.cpp<br>
@@ -0,0 +1,392 @@<br>
+/* -*- c++ -*- */<br>
+/*<br>
+ * Copyright © 2013 Intel Corporation<br>
+ *<br>
+ * Permission is hereby granted, free of charge, to any
person obtaining a<br>
+ * copy of this software and associated documentation
files (the "Software"),<br>
+ * to deal in the Software without restriction, including
without limitation<br>
+ * the rights to use, copy, modify, merge, publish,
distribute, sublicense,<br>
+ * and/or sell copies of the Software, and to permit
persons to whom the<br>
+ * Software is furnished to do so, subject to the
following conditions:<br>
+ *<br>
+ * The above copyright notice and this permission notice
(including the next<br>
+ * paragraph) shall be included in all copies or
substantial portions of the<br>
+ * Software.<br>
+ *<br>
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF
ANY KIND, EXPRESS OR<br>
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
OF MERCHANTABILITY,<br>
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
IN NO EVENT SHALL<br>
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
CLAIM, DAMAGES OR OTHER<br>
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
OTHERWISE, ARISING<br>
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
USE OR OTHER<br>
+ * DEALINGS IN THE SOFTWARE.<br>
+ */<br>
+<br>
+#include "ir_serialize.h"<br>
+<br>
+<br>
+/**<br>
+ * Wraps serialization of an ir instruction, writes
ir_type<br>
+ * and length of each instruction package as a header for
it<br>
+ */<br>
+void<br>
+ir_instruction::serialize(memory_writer &mem)<br>
+{<br>
+ uint32_t data_len = 0;<br>
+ uint8_t ir_type = this->ir_type;<br>
+ mem.write_uint8_t(ir_type);<br>
+<br>
+ int32_t start_pos = mem.position();<br>
+ mem.write_uint32_t(data_len);<br>
+<br>
+ this->serialize_data(mem);<br>
+<br>
+ data_len = mem.position() - start_pos -
sizeof(data_len);<br>
+ mem.overwrite(&data_len, sizeof(data_len),
start_pos);<br>
</blockquote>
<div><br>
</div>
<div>This function isn't checking the return values from
mem.write_*(), so there's no way for it to detect
failure. Also, since this function returns void, there's
no way for it to notify the caller of failure. A similar
comment applies to all of the other serialize*() functions
in this patch. (Of course, considering our previous
discussion about potentially removing these int return
values, this issue may be moot).<br>
</div>
<div> </div>
</div>
</div>
</div>
</blockquote>
<br>
I left error checking away from serialization but I left it there in
the memory_writer class to be available if some other possible user
would want it but it can be removed.<br>
<br>
<blockquote
cite="mid:CA+yLL66G3rZ19_PKKh0GaudfOH_bAzBJAEHoy+J2qHam3FxOrQ@mail.gmail.com"
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">
+}<br>
+<br>
+<br>
+<br>
+<br>
+static void<br>
+serialize_glsl_type(const glsl_type *type, memory_writer
&mem)<br>
</blockquote>
<div><br>
</div>
<div>The last time I reviewed this series, I mentioned the
idea of making a hashtable that maps each glsl_type to a
small integer, so that we could serialize each type just
once (see <a moz-do-not-send="true"
href="http://lists.freedesktop.org/archives/mesa-dev/2013-November/047740.html"
target="_blank">http://lists.freedesktop.org/archives/mesa-dev/2013-November/047740.html</a>).
At the time, it sounded like you liked that idea. Have
you made that change? It looks to me like you've stopped
serializing the built-in types, but user-defined types are
still serialized each time they occur.<br>
<br>
</div>
</div>
</div>
</div>
</blockquote>
<br>
I liked the idea but I did not manage to come up with a good
solution to iterate through builtin types at that time except
hardcoding. One possible way would be to extend glsl_symbol_table so
that one could iterate through types and come up with method to
recognize that type is a builtin type, I will try to get it working
from there. Currently only samplers are skipped, I took the check
against name away as you mentioned that interfaces might have same
exact name but different content.<br>
<br>
<br>
<blockquote
cite="mid:CA+yLL66G3rZ19_PKKh0GaudfOH_bAzBJAEHoy+J2qHam3FxOrQ@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div>With those two issues addressed, the patch is:<br>
<br>
Reviewed-by: Paul Berry <<a moz-do-not-send="true"
href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br>
</div>
</div>
</div>
</div>
</blockquote>
<br>
// Tapani<br>
<br>
</body>
</html>