<div dir="ltr">On 29 January 2014 01:24, 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">+<br>
+<br>
+/**<br>
+ * Serialization of exec_list, writes length of the list +<br>
+ * calls serialize_data for each instruction<br>
+ */<br>
+void<br>
+serialize_list(exec_list *list, memory_writer &mem)<br>
+{<br>
+ uint32_t list_len = 0;<br>
+ foreach_list(node, list)<br>
+ list_len++;<br>
+<br>
+ mem.write_uint32_t(list_len);<br>
+<br>
+ foreach_list(node, list)<br>
+ ((ir_instruction *)node)->serialize(mem);<br></blockquote><div><br></div><div>Nit pick: we could avoid having to walk the list twice by using mem.overwrite() to write out the length once we've written all the list elements. A similar comment applies to ir_call::serialize_data().<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>
+static void<br>
+serialize_glsl_type(const glsl_type *type, memory_writer &mem)<br>
+{<br>
+ uint32_t data_len = 0;<br>
+<br>
+ mem.write_string(type->name);<br>
+<br>
+ unsigned start_pos = mem.position();<br>
+ mem.write_uint32_t(data_len);<br>
+<br>
+ uint32_t type_hash;<br>
+<br>
+ /**<br>
+ * notify reader if a user defined type exists already<br>
+ * (has been serialized before)<br>
+ */<br>
+ uint8_t user_type_exists = 0;<br>
+<br>
+ /* serialize only user defined types */<br>
+ switch (type->base_type) {<br>
+ case GLSL_TYPE_ARRAY:<br>
+ case GLSL_TYPE_STRUCT:<br>
+ case GLSL_TYPE_INTERFACE:<br>
+ break;<br>
+ default:<br>
+ goto glsl_type_serilization_epilogue;<br>
+ }<br>
+<br></blockquote><div><br></div><div>With the changes I recommended in the last patch, I believe we can replace everything from here...<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">
+ type_hash = _mesa_hash_data(type, sizeof(glsl_type)); <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+ /* check if this type has been written before */<br>
+ if (mem.data_was_written((void *)type, type_hash))<br>
+ user_type_exists = 1;<br>
+ else<br>
+ mem.mark_data_written((void *)type, type_hash);<br></blockquote><div><br></div><div>...to here with:<br><br></div><div> user_type_exists = mem.make_unique_id(type, &type_hash);<br><br>Although I'd recommend renaming type_hash to type_id so that there's no confusion about the fact that it's a unique ID and not a hash (hashes aren't guaranteed to be unique).<br>
<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+ mem.write_uint8_t(user_type_exists);<br>
+ mem.write_uint32_t(type_hash);<br>
+<br>
+ /* no need to write again ... */<br>
+ if (user_type_exists)<br>
+ goto glsl_type_serilization_epilogue;<br></blockquote><div><br></div><div>Nit pick: how about instead of the goto, just do:<br><br></div><div>if (!user_type_exists) {<br></div><div> /* glsl type data */<br></div>
<div> mem.write_uint32_t((uint32_t) type->length);<br> ...<br>}<br><br></div><div>data_len = mem.position() - start_pos - sizeof(data_len);<br>...<br><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>
+ /* glsl type data */<br>
+ mem.write_uint32_t((uint32_t)type->length);<br>
+ mem.write_uint8_t((uint8_t)type->base_type);<br>
+ mem.write_uint8_t((uint8_t)type->interface_packing);<br>
+<br>
+ if (type->base_type == GLSL_TYPE_ARRAY)<br>
+ serialize_glsl_type(type->element_type(), mem);<br>
+ else if (type->base_type == GLSL_TYPE_STRUCT ||<br>
+ type->base_type == GLSL_TYPE_INTERFACE) {<br>
+ glsl_struct_field *field = type->fields.structure;<br>
+ for (unsigned k = 0; k < type->length; k++, field++) {<br>
+ mem.write_string(field->name);<br>
+ serialize_glsl_type(field->type, mem);<br>
+ mem.write_uint8_t((uint8_t)field->row_major);<br>
+ mem.write_int32_t(field->location);<br>
+ mem.write_uint8_t((uint8_t)field->interpolation);<br>
+ mem.write_uint8_t((uint8_t)field->centroid);<br>
+ }<br>
+ }<br>
+<br>
+glsl_type_serilization_epilogue:<br>
+<br>
+ data_len = mem.position() - start_pos - sizeof(data_len);<br>
+ mem.overwrite(&data_len, sizeof(data_len), start_pos);<br>
+}<br>
+<br>
+<br>
+void<br>
+ir_variable::serialize_data(memory_writer &mem)<br>
+{<br>
+ /* name can be NULL, see ir_print_visitor for explanation */<br>
+ const char *non_null_name = name ? name : "parameter";<br></blockquote><div><br></div><div>Since mem.write_string handles NULL strings, can we get rid of this?<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">
+ int64_t unique_id = (int64_t) (intptr_t) this;<br>
+ uint8_t mode = data.mode;<br>
+ uint8_t has_constant_value = constant_value ? 1 : 0;<br>
+ uint8_t has_constant_initializer = constant_initializer ? 1 : 0;<br>
+<br>
+ serialize_glsl_type(type, mem);<br>
+<br>
+ mem.write_string(non_null_name);<br>
+ mem.write_int64_t(unique_id);<br>
+ mem.write_uint8_t(mode);<br>
+<br>
+ mem.write(&data, sizeof(data));<br>
+<br>
+ mem.write_uint32_t(num_state_slots);<br>
+ mem.write_uint8_t(has_constant_value);<br>
+ mem.write_uint8_t(has_constant_initializer);<br>
+<br>
+ for (unsigned i = 0; i < num_state_slots; i++) {<br>
+ mem.write_int32_t(state_slots[i].swizzle);<br>
+ for (unsigned j = 0; j < 5; j++) {<br>
+ mem.write_int32_t(state_slots[i].tokens[j]);<br>
+ }<br>
+ }<br>
+<br>
+ if (constant_value)<br>
+ constant_value->serialize(mem);<br>
+<br>
+ if (constant_initializer)<br>
+ constant_initializer->serialize(mem);<br>
+<br>
+ uint8_t has_interface_type = get_interface_type() ? 1 : 0;<br>
+<br>
+ mem.write_uint8_t(has_interface_type);<br>
+ if (has_interface_type)<br>
+ serialize_glsl_type(get_interface_type(), mem);<br>
+}<br></blockquote><div><br></div><div>Everything's a nit pick except for the suggestion to use make_unique_id(). With that fixed, this patch is:<br><br>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br>
</div></div></div></div>