[Mesa-dev] [v2 3/6] glsl: ir_serializer class for the shader cache
Paul Berry
stereotype441 at gmail.com
Tue Nov 5 09:36:45 PST 2013
On 1 November 2013 02:16, Tapani Pälli <tapani.palli at intel.com> wrote:
> ir_serializer can serialize a gl_shader structure as binary
> data, will be used by OES_get_program_binary implementation.
>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> ---
> src/glsl/Makefile.sources | 1 +
> src/glsl/ir_cache_serializer.cpp | 933
> +++++++++++++++++++++++++++++++++++++++
> src/glsl/ir_cache_serializer.h | 207 +++++++++
> 3 files changed, 1141 insertions(+)
> create mode 100644 src/glsl/ir_cache_serializer.cpp
> create mode 100644 src/glsl/ir_cache_serializer.h
>
> diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
> index 2f7bfa1..0014f6f 100644
> --- a/src/glsl/Makefile.sources
> +++ b/src/glsl/Makefile.sources
> @@ -30,6 +30,7 @@ LIBGLSL_FILES = \
> $(GLSL_SRCDIR)/hir_field_selection.cpp \
> $(GLSL_SRCDIR)/ir_basic_block.cpp \
> $(GLSL_SRCDIR)/ir_builder.cpp \
> + $(GLSL_SRCDIR)/ir_cache_serializer.cpp \
> $(GLSL_SRCDIR)/ir_clone.cpp \
> $(GLSL_SRCDIR)/ir_constant_expression.cpp \
> $(GLSL_SRCDIR)/ir.cpp \
> diff --git a/src/glsl/ir_cache_serializer.cpp
> b/src/glsl/ir_cache_serializer.cpp
> new file mode 100644
> index 0000000..60e1d06
> --- /dev/null
> +++ b/src/glsl/ir_cache_serializer.cpp
> @@ -0,0 +1,933 @@
> +/* -*- c++ -*- */
> +/*
> + * Copyright © 2013 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> next
> + * paragraph) shall be included in all copies or substantial portions of
> the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include "ir_cache_serializer.h"
> +
> +
> +int
> +save_glsl_type(memory_writer &blob, const glsl_type *type)
> +{
> + uint32_t ir_len = 666;
> +
> + blob.write_string(type->name);
> +
> + int32_t start_pos = blob.position();
> + blob.write_uint32(&ir_len);
> +
> + /* actual glsl_type data */
> + uint32_t base_type = type->base_type;
> + uint32_t length = type->length;
> + uint8_t vector_elms = type->vector_elements;
> + uint8_t matrix_cols = type->matrix_columns;
> + uint8_t sampler_dimensionality = type->sampler_dimensionality;
> + uint8_t sampler_shadow = type->sampler_shadow;
> + uint8_t sampler_array = type->sampler_array;
> + uint8_t sampler_type = type->sampler_type;
> + uint8_t interface_packing = type->interface_packing;
> +
> + blob.write_uint32(&base_type);
>
There's no need for base_type to be 32 bits. It's a tiny enum.
> + blob.write_uint32(&length);
> + blob.write_uint8(&vector_elms);
> + blob.write_uint8(&matrix_cols);
> + blob.write_uint8(&sampler_dimensionality);
> + blob.write_uint8(&sampler_shadow);
> + blob.write_uint8(&sampler_array);
> + blob.write_uint8(&sampler_type);
> + blob.write_uint8(&interface_packing);
>
Two comments about this:
1. Having write_uint8 and write_uint32 take their arguments as pointers is
surprising, and it makes code like the above needlessly verbose.
I'd recommend changing write_uint8 and write_uint32 take their arguments as
values; then we can just write:
blob.write_uint32(type->base_type);
blob.write_uint32(type->length);
blob.write_uint8(vector_elms);
...and so on.
2. I'm not sure it makes sense to serialize all of these fields of
glsl_type. Really there are only 4 kinds of GLSL types: built-in types,
structs, interfaces, and arrays. Most of the fields above only matter for
built-in types. And for built-in types, we don't want to serialize the
entire glsl_type structure; we simply want to record which built-in type is
being referred to so that when the shader is loaded later, we can look up
one of the existing flyweights.
I have additional thoughts further below about saving GLSL types that would
make it unnecessary to save built-in types at all.
> +
> + if (type->base_type == GLSL_TYPE_ARRAY)
> + save_glsl_type(blob, type->element_type());
> + 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 < length; k++, field++) {
> + blob.write_string(field->name);
> + save_glsl_type(blob, field->type);
> + uint8_t row_major = field->row_major;
> + uint8_t interpolation = field->interpolation;
> + uint8_t centroid = field->centroid;
> + blob.write_uint8(&row_major);
> + blob.write_int32(&field->location);
> + blob.write_uint8(&interpolation);
> + blob.write_uint8(¢roid);
>
I'd like to make one more argument in favor of having the serialization and
deserialization functions in the classes themselves. If we did so, then we
could have a glsl_type::serialize() function as well as a
glsl_struct_field::serialize() function. Then the code above would
collapse into simply:
else if(type->base_type == GLSL_TYPE_STRUCT ||
type->base_type == GLSL_TYPE_INTERFACE) {
for (unsigned k = 0; k < length; k++)
type->fields.structure[k].serialize(blob);
}
> + }
> + }
> +
> + ir_len = blob.position() - start_pos - sizeof(ir_len);
> + blob.overwrite(&ir_len, sizeof(ir_len), start_pos);
> + return 0;
> +}
> +
> +
> +/**
> + * Function to create an unique string for a ir_variable. This is
> + * used by variable dereferences to indicate the exact ir_variable
> + * when deserialization happens.
>
I still don't understand why we don't just use the pointer for this
purpose. It's unique, and it takes up much less storage than
<name>_<decimal address>.
> + */
> +static char *_unique_name(ir_variable *var)
> +{
> + return ralloc_asprintf(NULL,
> + "%s_%ld", var->name ? var->name : "parameter", (intptr_t) var);
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_variable *ir)
> +{
> + /* name can be NULL, see ir_print_visitor for explanation */
> + const char *non_null_name = ir->name ? ir->name : "parameter";
> + char *uniq = _unique_name(ir);
> +
> + save_glsl_type(blob, ir->type);
>
It seems really wasteful to have the save_ir() functions for ir_variable,
ir_constant, ir_expression, ir_function_signature, and ir_texture all call
save_glsl_type(). The fact is that in any given GLSL program, there is a
small set of types that are used over and over again, and they are mostly
built-in types. Calling save_glsl_type() everywhere a type appears in the
IR is going to cause an enormous amount of bloat in the file size.
Here's an alternative idea:
1. While serializing, keep track of a hashable that maps each glsl_type to
a small integer. Instead of storing the entire type when we're
serializing, just store the small integers. While serializing, if we
encounter a type that's not in the hash table, assign it to the next
available small integer.
2. Preload this hashtable with a fixed static mapping for built-in types
(e.g. 0=float, 1=vec2, 2=vec3, 3=vec4, 4=int, and so on).
3. After serializing the IR for the shader, go through the hashtable and
serialize all of the non-built-in types with their associated integers.
4. When deserializing, read the hashtable mapping integers to types first.
Then, while deserializing the IR, each integer can just be looked up in the
hashtable to find the associated glsl_type.
This should cut dramatically down on storage size, which should make the
loading process much faster. It also avoids the need to store the built-in
types at all.
> +
> + blob.write_string(non_null_name);
> + blob.write_string(uniq);
> +
> + uint8_t mode = ir->mode;
> + uint8_t read_only = ir->read_only;
> + uint8_t centroid = ir->centroid;
> + uint8_t invariant = ir->invariant;
> + uint8_t interpolation = ir->interpolation;
> + uint8_t origin_upper_left = ir->origin_upper_left;
> + uint8_t pixel_center_integer = ir->pixel_center_integer;
> + uint8_t explicit_location = ir->explicit_location;
> + uint8_t explicit_index = ir->explicit_index;
> + uint8_t explicit_binding = ir->explicit_binding;
> + uint8_t has_initializer = ir->has_initializer;
> + int32_t depth_layout = ir->depth_layout;
> + uint8_t location_frac = ir->location_frac;
> + uint8_t has_constant_value = ir->constant_value ? 1 : 0;
> + uint8_t has_constant_initializer = ir->constant_initializer ? 1 : 0;
> +
> + blob.write_uint8(&mode);
> + blob.write_uint32(&ir->max_array_access);
> + blob.write_int32(&ir->location);
> + blob.write_uint8(&read_only);
> + blob.write_uint8(¢roid);
> + blob.write_uint8(&invariant);
> + blob.write_uint8(&interpolation);
> + blob.write_uint8(&origin_upper_left);
> + blob.write_uint8(&pixel_center_integer);
> + blob.write_uint8(&explicit_location);
> + blob.write_uint8(&explicit_index);
> + blob.write_uint8(&explicit_binding);
> + blob.write_uint8(&has_initializer);
> + blob.write_int32(&depth_layout);
> + blob.write_uint8(&location_frac);
> + blob.write_uint32(&ir->num_state_slots);
> + blob.write_uint8(&has_constant_value);
> + blob.write_uint8(&has_constant_initializer);
>
It seems backwards that in the in-memory representation (the ir_variable
class) we pack things like read_only, centroid, invariant, etc. tightly
into bitfields, but we serialize it out to a byte-aligned data structure.
Usually people try to make the serialized data format more tightly packed
than the in-memory format, because the tradeoffs for serialized data weigh
more strongly in favor of packing (access speed is less important; small
footprint is more important).
I think we should consider ways to maintain the tight bitfield packing in
the serialized data format. Here's one idea: how about if we refactor
ir_variable so that it contains a substructure called "bits", which
contains all the fields that are simple integers, packed carefully
together. Then, here in the serialization function, we just write out the
"bits" data structure verbatim using a single call to
memory_writer::write().
In addition to making the file size smaller and the
serialization/deserialization code simpler and faster, this would have the
advantage that if we add new fields to ir_variable::bits in the future, we
won't have to go to any effort to ensure that they will be serialized
correctly. The right thing will just happen automatically.
> +
> + for (unsigned i = 0; i < ir->num_state_slots; i++) {
> + blob.write_int32(&ir->state_slots[i].swizzle);
>
Swizzles are unsigned 8-bit values. This should be write_uint8.
> + for (unsigned j = 0; j < 5; j++) {
> + blob.write_int32(&ir->state_slots[i].tokens[j]);
>
Tokens are always either small integers or references to the
gl_state_index_ enum. So these can be stored as uint8's. (It might be
worth adding an assertion just to be on the safe side though).
> + }
> + }
> +
> + CACHE_DEBUG("save ir_variable [%s] [%s]\n", non_null_name, uniq);
> +
> + ralloc_free(uniq);
> +
> + if (ir->constant_value)
> + if (save(ir->constant_value))
> + return -1;
> +
> + if (ir->constant_initializer)
> + if (save(ir->constant_initializer))
> + return -1;
> +
> + uint8_t has_interface_type =
> + ir->is_interface_instance() ? 1 : 0;
> +
> + blob.write_uint8(&has_interface_type);
> +
> + if (has_interface_type)
> + save_glsl_type(blob, ir->get_interface_type());
>
We talked about this some previously, but I really think the convention of
having these serialization functions return "int" (with 0 indicating
success) is going to cause maintenance problems for us. Here's my
reasoning:
1. Since 0 means false we have to do mental gymnastics every time we see
something like:
if (save(ir->constant_value))
return -1;
to remind ourselves that the branch is taken on failure, not success.
2. If we really are trying to plan for a future where these ints can be
return codes instead of success/failure indicators, then doing this:
if (save(ir->constant_value))
return -1;
won't work anyway. We'd have to do something like this:
int retval = save(ir->constant_value));
if (retval)
return retval;
3. It's going to be very hard to remember to check the return values on
these functions. You yourself missed checking the return value here:
if (has_interface_type)
save_glsl_type(blob, ir->get_interface_type());
4. All of this is speculative future-proofing anyway. The only places in
ir_cache_serializer that I can find that would actually generate a nonzero
return code are: (a) if we find an unrecognized ir_type (would be better
handled by an assertion, or a pure virtual function, as I note below) and
(b) if memory allocation fails in memory_writer::grow() (which I thought we
agreed to remove).
I recommend ripping out all of these integer return types and returning
void. If, in the future, there are any legitimate error conditions that we
want to track, we should add an error state to memory_writer (or possibly
ir_serializer) and then check it at the end of serialization. That's the
technique we use for handling parsing and compilation errors elsewhere in
Mesa, and it's served us very well.
> +
> + return 0;
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_dereference_array *ir)
> +{
> + uint32_t array_type = ir->array->ir_type;
> + uint32_t array_index_type = ir->array_index->ir_type;
> + blob.write_uint32(&array_type);
> + save(ir->array);
> + blob.write_uint32(&array_index_type);
> + return save(ir->array_index);
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_dereference_record *ir)
> +{
> + uint32_t ir_record_type = ir->record->ir_type;
> + blob.write_string(ir->field);
> + blob.write_uint32(&ir_record_type);
> + return save(ir->record);
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_dereference_variable *ir)
> +{
> + char *uniq = _unique_name(ir->var);
> + blob.write_string(uniq);
> +
> + CACHE_DEBUG("save ir_dereference_variable [%s]\n", uniq);
> +
> + ralloc_free(uniq);
> + return 0;
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_constant *ir)
> +{
> + save_glsl_type(blob, ir->type);
> +
> + blob.write(&ir->value, sizeof(ir_constant_data));
> +
> + if (ir->array_elements) {
> + for (unsigned i = 0; i < ir->type->length; i++)
> + if (save(ir->array_elements[i]))
> + return -1;
> + }
> +
> + uint32_t components = 0;
> +
> + /* struct constant, dump components exec_list */
> + if (!ir->components.is_empty()) {
> + foreach_iter(exec_list_iterator, iter, ir->components)
> + components++;
> + blob.write_uint32(&components);
> +
> + foreach_iter(exec_list_iterator, iter, ir->components) {
> + ir_instruction *const inst = (ir_instruction *) iter.get();
> + if (save(inst))
> + return -1;
> + }
> + }
> +
> + return 0;
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_expression *ir)
> +{
> + int32_t num_operands = ir->get_num_operands();
> + uint32_t exp_operation = ir->operation;
> +
> + save_glsl_type(blob, ir->type);
>
Is it necessary to save the glsl_type for ir_expression nodes?
ir_expression has constructors which can infer the type based on the type
of the arguments.
> +
> + blob.write_uint32(&exp_operation);
> + blob.write_int32(&num_operands);
> +
> + /* operand ir_type below is written to make parsing easier */
> + for (unsigned i = 0; i < ir->get_num_operands(); i++) {
> + uint32_t ir_type = ir->operands[i]->ir_type;
> + blob.write_uint32(&ir_type);
> + if (save(ir->operands[i]))
> + return -1;
> + }
> + return 0;
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_function *ir)
> +{
> + uint32_t sig_amount = 0;
>
Can we rename this to "num_signatures"? "amount" usualy implies a quantity
that you don't measure by counting.
> +
> + foreach_iter(exec_list_iterator, iter, *ir)
> + sig_amount++;
> +
> + blob.write_string(ir->name);
> + blob.write_uint32(&sig_amount);
> +
> + CACHE_DEBUG("save ir_function [%s], %d sigs\n", ir->name, sig_amount);
> +
> + foreach_iter(exec_list_iterator, iter, *ir) {
> + ir_function_signature *const sig = (ir_function_signature *)
> iter.get();
> + if (save(sig))
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_function_signature *ir)
> +{
> + int32_t par_count = 0;
>
Can we rename this to something like "param_count"? It's not obvious what
"par" is an abbreviation for.
> + int32_t body_size = 0;
> + uint8_t is_builtin = ir->is_builtin();
> +
> + foreach_iter(exec_list_iterator, iter, ir->parameters)
> + par_count++;
> +
> + foreach_iter(exec_list_iterator, iter, ir->body)
> + body_size++;
> +
> + CACHE_DEBUG("signature (%s), returns %d, params %d size %d (builtin
> %d)\n",
> + ir->function_name(), ir->return_type->base_type, par_count,
> + body_size, is_builtin);
> +
> + blob.write_int32(&par_count);
> + blob.write_int32(&body_size);
> + blob.write_uint8(&is_builtin);
> +
> + /* dump the return type of function */
> + save_glsl_type(blob, ir->return_type);
> +
> + /* function parameters */
> + foreach_iter(exec_list_iterator, iter, ir->parameters) {
> + ir_variable *const inst = (ir_variable *) iter.get();
> + CACHE_DEBUG(" parameter %s\n", inst->name);
> + if (save(inst))
> + return -1;
> + }
> +
> + if (prototypes_only)
> + return 0;
>
I'm a little concerned about this. If we ever make a mistake where
prototypes_only doesn't match up properly between the serializer and the
deserializer, the deserializer will get hopelessly lost.
We could easily fix this by outputting a body_size of 0 whenever
prototypes_only is true.
> +
> + /* function body */
> + foreach_iter(exec_list_iterator, iter, ir->body) {
> + ir_instruction *const inst = (ir_instruction *) iter.get();
> + CACHE_DEBUG(" body instruction node type %d\n", inst->ir_type);
> + if (save(inst))
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_assignment *ir)
> +{
> + uint32_t write_mask = ir->write_mask;
> +
> + blob.write_uint32(&write_mask);
> +
> + /* lhs (ir_deference_*) */
> + uint32_t lhs_type = ir->lhs->ir_type;
> + blob.write_uint32(&lhs_type);
>
Isn't this redundant? The call to save(ir->lhs) just below will resolve to
ir_serializer::save(ir_instruction *), which writes out the ir_type as its
first action.
> +
> + if (save(ir->lhs))
> + return -1;
> +
> + if (ir->condition) {
> + CACHE_DEBUG("%s: assignment has condition, not supported",
> __func__);
>
Why don't we support this?
> + }
> +
> + /* rhs (constant, expression ...) */
> + uint32_t rhs_type = ir->rhs->ir_type;
> + blob.write_uint32(&rhs_type);
>
This also seems redundant.
> +
> + if (save(ir->rhs))
> + return -1;
> +
> + return 0;
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_return *ir)
> +{
> + ir_rvalue *const value = ir->get_value();
> + uint8_t has_rvalue = value ? 1 : 0;
> +
> + blob.write_uint8(&has_rvalue);
> +
> + if (has_rvalue) {
> + uint32_t ir_type = value->ir_type;
> + blob.write_uint32(&ir_type);
>
This is redundant too.
> + return save(value);
> + }
> + return 0;
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_swizzle *ir)
> +{
> + uint32_t components = ir->mask.num_components;
> + const uint32_t mask[4] = {
> + ir->mask.x,
> + ir->mask.y,
> + ir->mask.z,
> + ir->mask.w
> + };
> + uint32_t val_type = ir->val->ir_type;
> +
> + blob.write_uint32(&components);
> + blob.write(&mask, 4 * sizeof(mask[0]));
>
Why not just serialize the ir_swizzle_mask data structure in raw form? It
will take up less space and be faster.
> + blob.write_uint32(&val_type);
>
This is redundant for the reasons stated above. I'll stop commenting on
redundancies with ir_type now.
> +
> + return save(ir->val);
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_texture *ir)
> +{
> + int32_t op = ir->op;
> + uint8_t has_coordinate = ir->coordinate ? 1 : 0;
> + uint8_t has_projector = ir->projector ? 1 : 0;
> + uint8_t has_shadow_comp = ir->shadow_comparitor ? 1 : 0;
> + uint8_t has_offset = ir->offset ? 1 : 0;
> + uint32_t ir_type;
> +
> + CACHE_DEBUG("save_ir_texture: op %d, coord %d proj %d shadow %d\n",
> + op, has_coordinate, has_projector, has_shadow_comp);
> +
> + blob.write_int32(&op);
> + blob.write_uint8(&has_coordinate);
> + blob.write_uint8(&has_projector);
> + blob.write_uint8(&has_shadow_comp);
> + blob.write_uint8(&has_offset);
>
This would be another good candidate for creating a "bits" substructure, as
I recommended with ir_variable.
> +
> + save_glsl_type(blob, ir->type);
> +
> + /* sampler */
> + ir_type = ir->sampler->ir_type;
> + blob.write_uint32(&ir_type);
> + if (save(ir->sampler))
> + return -1;
> +
> + if (has_coordinate) {
> + ir_type = ir->coordinate->ir_type;
> + blob.write_uint32(&ir_type);
> + if (save(ir->coordinate))
> + return -1;
> + }
> +
> + if (has_projector) {
> + ir_type = ir->projector->ir_type;
> + blob.write_uint32(&ir_type);
> + if (save(ir->projector))
> + return -1;
> + }
> +
> + if (has_shadow_comp) {
> + ir_type = ir->shadow_comparitor->ir_type;
> + blob.write_uint32(&ir_type);
> + if (save(ir->shadow_comparitor))
> + return -1;
> + }
> +
> + if (has_offset) {
> + ir_type = ir->offset->ir_type;
> + blob.write_uint32(&ir_type);
> + if (save(ir->offset))
> + return -1;
> + }
> +
> + /* lod_info structure */
> + uint8_t has_lod = ir->lod_info.lod ? 1 : 0;
> + uint8_t has_bias = ir->lod_info.bias ? 1 : 0;
> + uint8_t has_sample_index = ir->lod_info.sample_index ? 1 : 0;
> + uint8_t has_component = ir->lod_info.component ? 1 : 0;
> + uint8_t has_dpdx = ir->lod_info.grad.dPdx ? 1 : 0;
> + uint8_t has_dpdy = ir->lod_info.grad.dPdy ? 1 : 0;
> +
> + blob.write_uint8(&has_lod);
> + blob.write_uint8(&has_bias);
> + blob.write_uint8(&has_sample_index);
> + blob.write_uint8(&has_component);
> + blob.write_uint8(&has_dpdx);
> + blob.write_uint8(&has_dpdy);
>
There are a number of places in the serializer where we have to go to extra
effort to serialize out a 1 or 0 to indicate whether an rvalue is present,
and then only serialize the rvalue if it is indeed present.
How about if instead we modify ir_serializer::save(ir_instruction *) so
that if it's passed a NULL pointer, it simply serializes out a magic
ir_type value that means "NULL"? (We could use ir_type_unset, which is
otherwise unused by the IR). This would reduce our risk of bugs because
there would be no danger of forgetting to include this logic in a place
where an ir_rvalue is allowed to be NULL.
> +
> + if (has_lod)
> + if (save(ir->lod_info.lod))
> + return -1;
> + if (has_bias)
> + if (save(ir->lod_info.bias))
> + return -1;
> + if (has_sample_index)
> + if (save(ir->lod_info.sample_index))
> + return -1;
> + if (has_component)
> + if (save(ir->lod_info.component))
> + return -1;
> + if (has_dpdx)
> + if (save(ir->lod_info.grad.dPdx))
> + return -1;
> + if (has_dpdy)
> + if (save(ir->lod_info.grad.dPdy))
> + return -1;
> +
> + return 0;
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_discard *ir)
> +{
> + uint8_t has_condition = ir->condition ? 1 : 0;
> + blob.write_uint8(&has_condition);
> +
> + if (ir->condition != NULL) {
> + CACHE_DEBUG("%s: error, there is no cond support here yet...\n",
> + __func__);
> + if (save(ir->condition))
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_call *ir)
> +{
> + blob.write_string(ir->callee_name());
> +
> + uint8_t has_return_deref = ir->return_deref ? 1 : 0;
> + uint8_t list_len = 0;
> + uint8_t use_builtin = ir->use_builtin;
> +
> + blob.write_uint8(&has_return_deref);
> +
> + if (ir->return_deref)
> + if (save(ir->return_deref))
> + return -1;
> +
> + /* call parameter list */
> + foreach_iter(exec_list_iterator, iter, *ir)
> + list_len++;
> +
> + blob.write_uint8(&list_len);
> +
> + foreach_iter(exec_list_iterator, iter, *ir) {
> + ir_instruction *const inst = (ir_instruction *) iter.get();
> +
> + int32_t ir_type = inst->ir_type;
> + blob.write_int32(&ir_type);
> +
> + if (save(inst))
> + return -1;
> + }
> +
> + blob.write_uint8(&use_builtin);
> +
> + return 0;
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_if *ir)
> +{
> + uint32_t then_len = 0, else_len = 0;
> + uint32_t cond_type = ir->condition->ir_type;
> +
> + /* then and else branch lengths */
> + foreach_iter(exec_list_iterator, iter, ir->then_instructions)
> + then_len++;
> + foreach_iter(exec_list_iterator, iter, ir->else_instructions)
> + else_len++;
> +
> + blob.write_uint32(&then_len);
> + blob.write_uint32(&else_len);
> + blob.write_uint32(&cond_type);
> +
> + CACHE_DEBUG("dump ir_if (then %d else %d), condition ir_type %d\n",
> + then_len, else_len, ir->condition->ir_type);
> +
> + save(ir->condition);
> +
> + /* dump branch instruction lists */
> + foreach_iter(exec_list_iterator, iter, ir->then_instructions) {
> + ir_instruction *const inst = (ir_instruction *) iter.get();
> + CACHE_DEBUG(" ir_if then instruction node type %d\n",
> inst->ir_type);
> + if (save(inst))
> + return -1;
> + }
> +
> + foreach_iter(exec_list_iterator, iter, ir->else_instructions) {
> + ir_instruction *const inst = (ir_instruction *) iter.get();
> + CACHE_DEBUG(" ir_if else instruction node type %d\n",
> inst->ir_type);
> + if (save(inst))
> + return -1;
> + }
>
I've seen a few places where this loop is repeated. How about if we make a
function whose job is to serialize an exec_list of ir_instructions
(including serializing its length)?
> +
> + return 0;
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_loop *ir)
> +{
> + uint8_t has_counter = ir->counter ? 1 : 0;
> + uint8_t has_from = ir->from ? 1 : 0;
> + uint8_t has_to = ir->to ? 1 : 0;
> + uint8_t has_incr = ir->increment ? 1 : 0;
> + uint32_t body_size = 0;
> + uint32_t ir_type;
> +
> + foreach_iter(exec_list_iterator, iter, ir->body_instructions)
> + body_size++;
> +
> + blob.write_uint8(&has_from);
> + blob.write_uint8(&has_to);
> + blob.write_uint8(&has_incr);
> + blob.write_uint8(&has_counter);
> + blob.write_int32(&ir->cmp);
> + blob.write_uint32(&body_size);
> +
> + if (has_from) {
> + ir_type = ir->from->ir_type;
> + blob.write_uint32(&ir_type);
> + if (save(ir->from))
> + return -1;
> + }
> +
> + if (has_to) {
> + ir_type = ir->to->ir_type;
> + blob.write_uint32(&ir_type);
> + if (save(ir->to))
> + return -1;
> + }
> +
> + if (has_incr) {
> + ir_type = ir->increment->ir_type;
> + blob.write_uint32(&ir_type);
> + if (save(ir->increment))
> + return -1;
> + }
> +
> + if (has_counter) {
> + blob.write_string(ir->counter->name);
> + if (save(ir->counter))
> + return -1;
> + }
> +
> + foreach_iter(exec_list_iterator, iter, ir->body_instructions) {
> + ir_instruction *const inst = (ir_instruction *) iter.get();
> + CACHE_DEBUG("save loop instruction type %d\n", inst->ir_type);
> + if (save(inst))
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> +int ir_serializer::save_ir(ir_loop_jump *ir)
> +{
> + uint32_t mode = ir->mode;
> + return blob.write_uint32(&mode);
>
mode is a tiny enum and should be serialized as a uint8.
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_emit_vertex *ir)
> +{
> + return 0;
> +}
> +
> +
> +int
> +ir_serializer::save_ir(ir_end_primitive *ir)
> +{
> + return 0;
> +}
> +
> +
> +/**
> + * writes instruction type, packet size and calls
> + * save function for the instruction to save the data
> + */
> +int
> +ir_serializer::save(ir_instruction *ir)
> +{
> + uint32_t ir_len = 666;
> + uint32_t ir_type = ir->ir_type;
> +
> + blob.write_uint32(&ir_type);
>
ir_type is a small enum and should be serialized as a uint8.
> +
> + int32_t start_pos = blob.position();
> +
> + blob.write_uint32(&ir_len);
> +
> +#define SAVE_IR(type)\
> + if (save_ir(static_cast<type *>(ir))) goto write_errors;
> +
> + switch(ir->ir_type) {
> +
> + case ir_type_variable:
> + SAVE_IR(ir_variable);
> + break;
> + case ir_type_call:
> + SAVE_IR(ir_call);
> + break;
> + case ir_type_constant:
> + SAVE_IR(ir_constant);
> + break;
> + case ir_type_discard:
> + SAVE_IR(ir_discard);
> + break;
> + case ir_type_expression:
> + SAVE_IR(ir_expression);
> + break;
> + case ir_type_dereference_array:
> + SAVE_IR(ir_dereference_array);
> + break;
> + case ir_type_dereference_record:
> + SAVE_IR(ir_dereference_record);
> + break;
> + case ir_type_dereference_variable:
> + SAVE_IR(ir_dereference_variable);
> + break;
> + case ir_type_function:
> + SAVE_IR(ir_function);
> + break;
> + case ir_type_function_signature:
> + SAVE_IR(ir_function_signature);
> + break;
> + case ir_type_swizzle:
> + SAVE_IR(ir_swizzle);
> + break;
> + case ir_type_texture:
> + SAVE_IR(ir_texture);
> + break;
> + case ir_type_assignment:
> + SAVE_IR(ir_assignment);
> + break;
> + case ir_type_if:
> + SAVE_IR(ir_if);
> + break;
> + case ir_type_loop:
> + SAVE_IR(ir_loop);
> + break;
> + case ir_type_loop_jump:
> + SAVE_IR(ir_loop_jump);
> + break;
> + case ir_type_return:
> + SAVE_IR(ir_return);
> + break;
> + case ir_type_emit_vertex:
> + SAVE_IR(ir_emit_vertex);
> + break;
> + case ir_type_end_primitive:
> + SAVE_IR(ir_end_primitive);
> + break;
> +
> + default:
> + CACHE_DEBUG("%s: error, type %d not implemented\n",
> + __func__, ir->ir_type);
> + return -1;
> + }
>
Another advantage of moving the serialization logic into the IR classes is
that we could do this dispatch by a pure virtual function rather than a
switch statement. Then there would be no need to have a default case to do
error handling if we encounter an unexpected ir_type. Instead, the
compiler would check that each type of IR had a corresponding
implementation of the pure virtual.
> +
> + ir_len = blob.position() - start_pos - sizeof(ir_len);
> +
> + blob.overwrite(&ir_len, sizeof(ir_len), start_pos);
> +
> + return 0;
> +
> +write_errors:
> + CACHE_DEBUG("%s: write errors (ir type %d)\n", __func__, ir->ir_type);
> + return -1;
> +}
> +
> +
> +static void
> +_write_header(gl_shader *shader, const char *mesa_sha, memory_writer
> &blob)
> +{
> + GET_CURRENT_CONTEXT(ctx);
> +
> + blob.write_string(mesa_sha);
> + blob.write_string((const char *)ctx->Driver.GetString(ctx, GL_VENDOR));
> + blob.write_string((const char *)ctx->Driver.GetString(ctx,
> GL_RENDERER));
> + blob.write_uint32(&shader->Version);
> + blob.write_uint32(&shader->Type);
> + blob.write_uint8(&shader->IsES);
> +
> + /* post-link data */
> + blob.write_uint32(&shader->num_samplers);
> + blob.write_uint32(&shader->active_samplers);
> + blob.write_uint32(&shader->shadow_samplers);
> + blob.write_uint32(&shader->num_uniform_components);
> + blob.write_uint32(&shader->num_combined_uniform_components);
> +
> + for (unsigned i = 0; i < MAX_SAMPLERS; i++)
> + blob.write_uint8(&shader->SamplerUnits[i]);
> +
> + for (unsigned i = 0; i < MAX_SAMPLERS; i++) {
> + int32_t target = shader->SamplerTargets[i];
> + blob.write_int32(&target);
> + }
> +}
> +
> +
> +static void
> +_dump_bool(bool value, memory_writer &blob)
> +{
> + uint8_t val = value;
> + blob.write_uint8(&val);
> +}
>
This seems like it should be in the memory_writer class as
void memory_writer::write_bool(bool value);
> +
> +
> +/**
> + * some of the state such as extension bits is required from
> + * the preprocessing stage, this is when caching unlinked shaders
> + */
> +static void
> +_write_state(struct _mesa_glsl_parse_state *state, memory_writer &blob)
> +{
> + blob.write_uint32(&state->language_version);
> +
> + _dump_bool(state->ARB_draw_buffers_enable, blob);
> + _dump_bool(state->ARB_draw_buffers_warn, blob);
> + _dump_bool(state->ARB_draw_instanced_enable, blob);
> + _dump_bool(state->ARB_draw_instanced_warn, blob);
> + _dump_bool(state->ARB_explicit_attrib_location_enable, blob);
> + _dump_bool(state->ARB_explicit_attrib_location_warn, blob);
> + _dump_bool(state->ARB_fragment_coord_conventions_enable, blob);
> + _dump_bool(state->ARB_fragment_coord_conventions_warn, blob);
> + _dump_bool(state->ARB_texture_rectangle_enable, blob);
> + _dump_bool(state->ARB_texture_rectangle_warn, blob);
> + _dump_bool(state->EXT_texture_array_enable, blob);
> + _dump_bool(state->EXT_texture_array_warn, blob);
> + _dump_bool(state->ARB_shader_texture_lod_enable, blob);
> + _dump_bool(state->ARB_shader_texture_lod_warn, blob);
> + _dump_bool(state->ARB_shader_stencil_export_enable, blob);
> + _dump_bool(state->ARB_shader_stencil_export_warn, blob);
> + _dump_bool(state->AMD_conservative_depth_enable, blob);
> + _dump_bool(state->AMD_conservative_depth_warn, blob);
> + _dump_bool(state->ARB_conservative_depth_enable, blob);
> + _dump_bool(state->ARB_conservative_depth_warn, blob);
> + _dump_bool(state->AMD_shader_stencil_export_enable, blob);
> + _dump_bool(state->AMD_shader_stencil_export_warn, blob);
> + _dump_bool(state->OES_texture_3D_enable, blob);
> + _dump_bool(state->OES_texture_3D_warn, blob);
> + _dump_bool(state->OES_EGL_image_external_enable, blob);
> + _dump_bool(state->OES_EGL_image_external_warn, blob);
> + _dump_bool(state->ARB_shader_bit_encoding_enable, blob);
> + _dump_bool(state->ARB_shader_bit_encoding_warn, blob);
> + _dump_bool(state->ARB_uniform_buffer_object_enable, blob);
> + _dump_bool(state->ARB_uniform_buffer_object_warn, blob);
> + _dump_bool(state->OES_standard_derivatives_enable, blob);
> + _dump_bool(state->OES_standard_derivatives_warn, blob);
> + _dump_bool(state->ARB_texture_cube_map_array_enable, blob);
> + _dump_bool(state->ARB_texture_cube_map_array_warn, blob);
> + _dump_bool(state->ARB_shading_language_packing_enable, blob);
> + _dump_bool(state->ARB_shading_language_packing_warn, blob);
> + _dump_bool(state->ARB_texture_multisample_enable, blob);
> + _dump_bool(state->ARB_texture_multisample_warn, blob);
> + _dump_bool(state->ARB_texture_query_lod_enable, blob);
> + _dump_bool(state->ARB_texture_query_lod_warn, blob);
> + _dump_bool(state->ARB_gpu_shader5_enable, blob);
> + _dump_bool(state->ARB_gpu_shader5_warn, blob);
> + _dump_bool(state->AMD_vertex_shader_layer_enable, blob);
> + _dump_bool(state->AMD_vertex_shader_layer_warn, blob);
> + _dump_bool(state->ARB_shading_language_420pack_enable, blob);
> + _dump_bool(state->ARB_shading_language_420pack_warn, blob);
> + _dump_bool(state->EXT_shader_integer_mix_enable, blob);
> + _dump_bool(state->EXT_shader_integer_mix_warn, blob);
>
This is another case where I'm concerned about us forgetting to update this
code when we add new extension flags. I think this would be a good
candidate for the "bits" idea that I talked about above with ir_variable.
Although in this case I might call the structure something like "enables"
rather than "bits".
> +}
> +
> +
> +/**
> + * serializes a single gl_shader, writes shader header
> + * information and exec_list of instructions
> + */
> +char *
> +ir_serializer::serialize(struct gl_shader *shader,
> + struct _mesa_glsl_parse_state *state,
> + const char *mesa_sha, size_t *size)
> +{
> + uint32_t total = 0;
> +
> + prototypes_only = true;
> +
> + *size = 0;
> +
> + int32_t start_pos = blob.position();
> + uint32_t shader_data_len = 666;
> + uint32_t shader_type = shader->Type;
> +
> + blob.write_uint32(&shader_data_len);
> + blob.write_uint32(&shader_type);
> +
> + _write_header(shader, mesa_sha, blob);
> +
> + if (state)
> + _write_state(state, blob);
> +
> + /* count variables + functions and dump prototypes */
> + foreach_list_const(node, shader->ir) {
> + if (((ir_instruction *) node)->as_variable())
> + total++;
> + if (((ir_instruction *) node)->as_function())
> + total++;
> + }
> +
> + blob.write_uint32(&total);
> +
> + CACHE_DEBUG("write %d prototypes\n", total);
> +
> + foreach_list_const(node, shader->ir) {
> + ir_instruction *const inst = (ir_instruction *) node;
> + if (inst->as_variable())
> + if (save(inst))
> + goto write_errors;
> + }
> +
> + foreach_list_const(node, shader->ir) {
> + ir_instruction *const inst = (ir_instruction *) node;
> + if (inst->as_function())
> + if (save(inst))
> + goto write_errors;
> + }
>
Why is it necessary to save the variables and instructions first?
> +
> + /* all shader instructions */
> + prototypes_only = false;
> + foreach_list_const(node, shader->ir) {
> + ir_instruction *instruction = (ir_instruction *) node;
> + if (save(instruction))
> + goto write_errors;
> + }
> +
> + CACHE_DEBUG("cached a shader\n");
> +
> + /* how much has been written */
> + *size = blob.position();
> +
> + shader_data_len = blob.position() -
> + start_pos - sizeof(shader_data_len);
> + blob.overwrite(&shader_data_len, sizeof(shader_data_len), start_pos);
> +
> + return blob.release_memory(size);
> +
> +write_errors:
> +
> + return NULL;
> +}
> diff --git a/src/glsl/ir_cache_serializer.h
> b/src/glsl/ir_cache_serializer.h
> new file mode 100644
> index 0000000..6e33ca9
> --- /dev/null
> +++ b/src/glsl/ir_cache_serializer.h
> @@ -0,0 +1,207 @@
> +/* -*- c++ -*- */
> +/*
> + * Copyright © 2013 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> next
> + * paragraph) shall be included in all copies or substantial portions of
> the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +#pragma once
> +#ifndef IR_CACHE_SERIALIZER_H
> +#define IR_CACHE_SERIALIZER_H
> +
> +#include "ir.h"
> +#include "glsl_parser_extras.h"
> +
> +#ifdef SHADER_CACHE_DEBUG
> +#define CACHE_DEBUG(fmt, ...) printf(fmt, ## __VA_ARGS__)
> +#else
> +#define CACHE_DEBUG(fmt, ...) do {} while (0)
> +#endif
> +
> +#ifdef __cplusplus
> +/**
> + * Helper class for writing data to memory
> + *
> + * This class maintains a dynamically-sized memory buffer and allows
> + * for data to be efficiently appended to it with automatic resizing.
> + */
> +class memory_writer
> +{
> +public:
> + memory_writer() :
> + memory(NULL),
> + curr_size(0),
> + pos(0) {}
> +
> + ~memory_writer()
> + {
> + free(memory);
> + }
> +
> + /* user wants to claim the memory */
> + char *release_memory(size_t *size)
> + {
> + /* final realloc to free allocated but unused memory */
> + char *result = (char *) realloc(memory, pos);
> + memory = NULL;
> + curr_size = 0;
> + pos = 0;
> + return result;
> + }
> +
> + /* write functions for different data types */
> + int write_uint8(uint8_t *data)
> + {
> + return write(data, sizeof(uint8_t));
> + }
> + int write_int32(int32_t *data)
> + {
> + return write(data, sizeof(int32_t));
> + }
> + int write_uint32(uint32_t *data)
> + {
> + return write(data, sizeof(uint32_t));
> + }
> +
> + /* write function that reallocates more memory if required */
> + int write(const void *data, int32_t size)
> + {
> + if (!memory || pos > (int32_t)(curr_size - size))
> + if (grow(size))
> + return -1;
> +
> + memcpy(memory + pos, data, size);
> +
> + pos += size;
> + return 0;
> + }
> +
> + int overwrite(const void *data, int32_t size, int32_t offset)
> + {
> + if (offset < 0 || offset + size > pos)
> + return -1;
> + memcpy(memory + offset, data, size);
> + return 0;
> + }
> +
> + int write_string(const char *str)
> + {
> + if (!str)
> + return -1;
> + char terminator = '\0';
> + write(str, strlen(str));
> + write(&terminator, 1);
> + return 0;
> + }
> +
> + inline int32_t position() { return pos; }
> +
> +private:
> +
> + /* reallocate more memory */
> + int grow(int32_t size)
> + {
> + int32_t new_size = 2 * (curr_size + size);
> + char *more_mem = (char *) realloc(memory, new_size);
> + if (more_mem == NULL) {
> + free(memory);
> + memory = NULL;
> + return -1;
> + } else {
> + memory = more_mem;
> + curr_size = new_size;
> + return 0;
> + }
> + }
> +
> + /* allocated memory */
> + char *memory;
> +
> + /* current size of the whole allocation */
> + int32_t curr_size;
> +
> + /* write position / size of the data written */
> + int32_t pos;
> +};
> +
> +
> +/**
> + * Utility function used both by ir_serializer and
> + * gl_shader_program structure serialization code
> + */
> +int save_glsl_type(memory_writer &blob, const glsl_type *type);
> +
> +/**
> + * Class to serialize gl_shader as a binary data blob
> + *
> + * Serialization is done by writing small header data section, parser
> + * state data and all IR instructions of the shader including any relevant
> + * data to be able to deserialize them back.
> + */
> +class ir_serializer
> +{
> +public:
> + ir_serializer() :
> + prototypes_only(false)
> + {
> + }
> +
> + /* serialize gl_shader to memory */
> + char *serialize(struct gl_shader *shader,
> + struct _mesa_glsl_parse_state *state,
> + const char *mesa_sha, size_t *size);
> +
> +private:
> +
> + memory_writer blob;
> +
> + bool prototypes_only;
> +
> + /**
> + * writes ir_type and instruction dump size as a 'header'
> + * for each instruction before calling save_ir with it
> + */
> + int save(ir_instruction *ir);
> +
> + /* methods for each ir type */
> + int save_ir(ir_variable *ir);
> + int save_ir(ir_assignment *ir);
> + int save_ir(ir_call *ir);
> + int save_ir(ir_constant *ir);
> + int save_ir(ir_dereference_array *ir);
> + int save_ir(ir_dereference_record *ir);
> + int save_ir(ir_dereference_variable *ir);
> + int save_ir(ir_discard *ir);
> + int save_ir(ir_expression *ir);
> + int save_ir(ir_function *ir);
> + int save_ir(ir_function_signature *ir);
> + int save_ir(ir_if *ir);
> + int save_ir(ir_loop *ir);
> + int save_ir(ir_loop_jump *ir);
> + int save_ir(ir_return *ir);
> + int save_ir(ir_swizzle *ir);
> + int save_ir(ir_texture *ir);
> + int save_ir(ir_emit_vertex *ir);
> + int save_ir(ir_end_primitive *ir);
> +
> +};
> +#endif /* ifdef __cplusplus */
> +
> +#endif /* IR_CACHE_SERIALIZER_H */
> --
> 1.8.1.4
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131105/62f5760f/attachment-0001.html>
More information about the mesa-dev
mailing list