<div dir="ltr">On 2 January 2014 03:58, 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">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><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 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>With those two issues addressed, the patch is:<br><br>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br>
</div></div></div></div>