Re: [Mesa-dev] [PATCH 02/10] glsl: serialize methods for IR instructions
On 29 January 2014 01:24, Tapani Pälli wrote:
> +
> +
> +/**
> + * Serialization of exec_list, writes length of the list +
> + * calls serialize_data for each instruction
> + */
> +void
> +serialize_list(exec_list *list, memory_writer &mem)
> +{
> + uint32_t list_len = 0;
> + foreach_list(node, list)
> + list_len++;
> +
> + mem.write_uint32_t(list_len);
> +
> + foreach_list(node, list)
> + ((ir_instruction *)node)->serialize(mem);
>
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().
> +}
> +
> +
> +static void
> +serialize_glsl_type(const glsl_type *type, memory_writer &mem)
> +{
> + uint32_t data_len = 0;
> +
> + mem.write_string(type->name);
> +
> + unsigned start_pos = mem.position();
> + mem.write_uint32_t(data_len);
> +
> + uint32_t type_hash;
> +
> + /**
> +* notify reader if a user defined type exists already
> +* (has been serialized before)
> +*/
> + uint8_t user_type_exists = 0;
> +
> + /* serialize only user defined types */
> + switch (type->base_type) {
> + case GLSL_TYPE_ARRAY:
> + case GLSL_TYPE_STRUCT:
> + case GLSL_TYPE_INTERFACE:
> + break;
> + default:
> + goto glsl_type_serilization_epilogue;
> + }
> +
>
With the changes I recommended in the last patch, I believe we can replace
everything from here...
> + type_hash = _mesa_hash_data(type, sizeof(glsl_type));
>
+
> + /* check if this type has been written before */
> + if (mem.data_was_written((void *)type, type_hash))
> + user_type_exists = 1;
> + else
> + mem.mark_data_written((void *)type, type_hash);
>
...to here with:
user_type_exists = mem.make_unique_id(type, &type_hash);
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).
> +
> + mem.write_uint8_t(user_type_exists);
> + mem.write_uint32_t(type_hash);
> +
> + /* no need to write again ... */
> + if (user_type_exists)
> + goto glsl_type_serilization_epilogue;
>
Nit pick: how about instead of the goto, just do:
if (!user_type_exists) {
/* glsl type data */
mem.write_uint32_t((uint32_t) type->length);
...
}
data_len = mem.position() - start_pos - sizeof(data_len);
...
> +
> + /* glsl type data */
> + mem.write_uint32_t((uint32_t)type->length);
> + mem.write_uint8_t((uint8_t)type->base_type);
> + mem.write_uint8_t((uint8_t)type->interface_packing);
> +
> + if (type->base_type == GLSL_TYPE_ARRAY)
> + serialize_glsl_type(type->element_type(), mem);
> + else if (type->base_type == GLSL_TYPE_STRUCT ||
> + type->base_type == GLSL_TYPE_INTERFACE) {
> + glsl_struct_field *field = type->fields.structure;
> + for (unsigned k = 0; k < type->length; k++, field++) {
> + mem.write_string(field->name);
> + serialize_glsl_type(field->type, mem);
> + mem.write_uint8_t((uint8_t)field->row_major);
> + mem.write_int32_t(field->location);
> + mem.write_uint8_t((uint8_t)field->interpolation);
> + mem.write_uint8_t((uint8_t)field->centroid);
> + }
> + }
> +
> +glsl_type_serilization_epilogue:
> +
> + data_len = mem.position() - start_pos - sizeof(data_len);
> + mem.overwrite(&data_len, sizeof(data_len), start_pos);
> +}
> +
> +
> +void
> +ir_variable::serialize_data(memory_writer &mem)
> +{
> + /* name can be NULL, see ir_print_visitor for explanation */
> + const char *non_null_name = name ? name : "parameter";
>
Since mem.write_string handles NULL strings, can we get rid of this?
> + int64_t unique_id = (int64_t) (intptr_t) this;
> + uint8_t mode = data.mode;
> + uint8_t has_constant_value = constant_value ? 1 : 0;
> + uint8_t has_constant_initializer = constant_initializer ? 1 : 0;
> +
> + serialize_glsl_type(type, mem);
> +
> + mem.write_string(non_null_name);
> + mem.write_int64_t(unique_id);
> + mem.write_uint8_t(mode);
> +
> + mem.write(&data, sizeof(data));
> +
> + mem.write_uint32_t(num_state_slots);
> + mem.write_uint8_t(has_constant_value);
> + mem.write_uint8_t(has_constant_initializer);
> +
> + for (unsigned i = 0; i < num_state_slots; i++) {
> + mem.write_int32_t(state_slots[i].swizzle);
> + for (unsigned j = 0; j < 5; j++) {
> + mem.write_int32_t(state_slots[i].tokens[j]);
> + }
> + }
> +
> + if (constant_value)
> + constant_value->serialize(mem);
> +
> + if (constant_initializer)
> + constant_initializer->serialize(mem);
> +
> + uint8_t has_interface_type = get_interface_type() ? 1 : 0;
> +
> + mem.write_uint8_t(has_interface_type);
> + if (has_interface_type)
> + serialize_glsl_type(get_interface_type(), mem);
> +}
>
Everything's a nit pick except for the suggestion to use make_unique_id().
With that fixed, this patch is
[Mesa-dev] [PATCH 02/10] glsl: serialize methods for IR instructions
Patch adds a new virtual function for ir_instruction base class which
has to be implemented by each instruction. This data will be used by the
shader binary cache implementation.
Signed-off-by: Tapani Pälli
---
src/glsl/Makefile.sources | 1 +
src/glsl/ir.h | 45 +
src/glsl/ir_serialize.cpp | 407 ++
src/glsl/ir_serialize.h | 36
4 files changed, 489 insertions(+)
create mode 100644 src/glsl/ir_serialize.cpp
create mode 100644 src/glsl/ir_serialize.h
diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
index e69c1ac..24bb70f 100644
--- a/src/glsl/Makefile.sources
+++ b/src/glsl/Makefile.sources
@@ -45,6 +45,7 @@ LIBGLSL_FILES = \
$(GLSL_SRCDIR)/ir_reader.cpp \
$(GLSL_SRCDIR)/ir_rvalue_visitor.cpp \
$(GLSL_SRCDIR)/ir_set_program_inouts.cpp \
+ $(GLSL_SRCDIR)/ir_serialize.cpp \
$(GLSL_SRCDIR)/ir_validate.cpp \
$(GLSL_SRCDIR)/ir_variable_refcount.cpp \
$(GLSL_SRCDIR)/linker.cpp \
diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index 19e8383..1e31d70 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -35,6 +35,7 @@
#include "ir_visitor.h"
#include "ir_hierarchical_visitor.h"
#include "main/mtypes.h"
+#include "memory_writer.h"
#ifdef __cplusplus
@@ -107,6 +108,10 @@ public:
/** ir_print_visitor helper for debugging. */
void print(void) const;
+ /* serialization */
+ void serialize(memory_writer &mem);
+ virtual void serialize_data(memory_writer &mem) = 0;
+
virtual void accept(ir_visitor *) = 0;
virtual ir_visitor_status accept(ir_hierarchical_visitor *) = 0;
virtual ir_instruction *clone(void *mem_ctx,
@@ -167,6 +172,8 @@ public:
virtual ir_rvalue *clone(void *mem_ctx, struct hash_table *) const;
+ virtual void serialize_data(memory_writer &mem);
+
virtual void accept(ir_visitor *v)
{
v->visit(this);
@@ -379,6 +386,8 @@ public:
return this;
}
+ virtual void serialize_data(memory_writer &mem);
+
virtual void accept(ir_visitor *v)
{
v->visit(this);
@@ -769,6 +778,8 @@ public:
ir_function_signature *clone_prototype(void *mem_ctx,
struct hash_table *ht) const;
+ virtual void serialize_data(memory_writer &mem);
+
virtual void accept(ir_visitor *v)
{
v->visit(this);
@@ -900,6 +911,8 @@ public:
return this;
}
+ virtual void serialize_data(memory_writer &mem);
+
virtual void accept(ir_visitor *v)
{
v->visit(this);
@@ -974,6 +987,8 @@ public:
return this;
}
+ virtual void serialize_data(memory_writer &mem);
+
virtual void accept(ir_visitor *v)
{
v->visit(this);
@@ -998,6 +1013,8 @@ public:
virtual ir_loop *clone(void *mem_ctx, struct hash_table *ht) const;
+ virtual void serialize_data(memory_writer &mem);
+
virtual void accept(ir_visitor *v)
{
v->visit(this);
@@ -1033,6 +1050,8 @@ public:
virtual ir_constant *constant_expression_value(struct hash_table
*variable_context = NULL);
+ virtual void serialize_data(memory_writer &mem);
+
virtual void accept(ir_visitor *v)
{
v->visit(this);
@@ -1459,6 +1478,8 @@ public:
*/
static ir_expression_operation get_operator(const char *);
+ virtual void serialize_data(memory_writer &mem);
+
virtual void accept(ir_visitor *v)
{
v->visit(this);
@@ -1497,6 +1518,8 @@ public:
return this;
}
+ virtual void serialize_data(memory_writer &mem);
+
virtual void accept(ir_visitor *v)
{
v->visit(this);
@@ -1583,6 +1606,8 @@ public:
return value;
}
+ virtual void serialize_data(memory_writer &mem);
+
virtual void accept(ir_visitor *v)
{
v->visit(this);
@@ -1617,6 +1642,8 @@ public:
virtual ir_loop_jump *clone(void *mem_ctx, struct hash_table *) const;
+ virtual void serialize_data(memory_writer &mem);
+
virtual void accept(ir_visitor *v)
{
v->visit(this);
@@ -1657,6 +1684,8 @@ public:
virtual ir_discard *clone(void *mem_ctx, struct hash_table *ht) const;
+ virtual void serialize_data(memory_writer &mem);
+
virtual void accept(ir_visitor *v)
{
v->visit(this);
@@ -1729,6 +1758,8 @@ public:
virtual ir_constant *constant_expression_value(struct hash_table
*variable_context = NULL);
+ virtual void serialize_data(memory_writer &mem);
+
virtual void accept(ir_visitor *v)
{
v->visit(this);
@@ -1840,6 +1871,8 @@ public:
*/
static ir_swizzle *create(ir_rvalue *, const char *, unsigned
vector_length);
+ virtual void serialize_data(memory_writer &mem);
+
virtual void accept(ir_visitor *v)
{
v->visit(this);
@@ -1943,6 +1976,8 @@ public:
return this->var;
}
+ virtual void serialize_data(memory_writer &mem);
+
virtual void accept(ir_visitor *v)
{
v->visit(this);
@@ -1992,6 +2027,8 @@ public:
