[Mesa-dev] [PATCH 08/37] glsl: add initial implementation of shader cache

[Tapani Pälli]

Hi Timothy;

On 01/24/2017 01:12 AM, Timothy Arceri wrote:
> From: Timothy Arceri <timothy.arceri at collabora.com>
>

8<

> +
> +static void
> +write_uniforms(struct blob *metadata, struct gl_shader_program *prog)
> +{
> +   blob_write_uint32(metadata, prog->SamplersValidated);
> +   blob_write_uint32(metadata, prog->data->NumUniformStorage);
> +   blob_write_uint32(metadata, prog->data->NumUniformDataSlots);
> +
> +   for (unsigned i = 0; i < prog->data->NumUniformStorage; i++) {
> +      encode_type_to_blob(metadata, prog->data->UniformStorage[i].type);
> +      blob_write_uint32(metadata, prog->data->UniformStorage[i].array_elements);
> +      blob_write_string(metadata, prog->data->UniformStorage[i].name);
> +      blob_write_uint32(metadata, prog->data->UniformStorage[i].storage -
> +                                  prog->data->UniformDataSlots);
> +      blob_write_uint32(metadata, prog->data->UniformStorage[i].remap_location);
> +      blob_write_uint32(metadata, prog->data->UniformStorage[i].block_index);
> +      blob_write_uint32(metadata, prog->data->UniformStorage[i].atomic_buffer_index);
> +      blob_write_uint32(metadata, prog->data->UniformStorage[i].offset);
> +      blob_write_uint32(metadata, prog->data->UniformStorage[i].array_stride);
> +      blob_write_uint32(metadata, prog->data->UniformStorage[i].matrix_stride);
> +      blob_write_uint32(metadata, prog->data->UniformStorage[i].row_major);
> +      blob_write_uint32(metadata,
> +                        prog->data->UniformStorage[i].num_compatible_subroutines);
> +      blob_write_uint32(metadata,
> +                        prog->data->UniformStorage[i].top_level_array_size);
> +      blob_write_uint32(metadata,
> +                        prog->data->UniformStorage[i].top_level_array_stride);
> +   }
> +}

I realize I'm repeating myself here but please do consider read/write a 
complete structure instead of individual fields whenever possible (this 
can be applied to many structures in this patch set). The reason is not 
just to optimize, it is to make this code more maintainable and robust 
so that it won't break when these structures change. Ideally cache code 
does not have to be touched when that happens. Cache can contain sizes 
of structures so that we can just simply remove entries that were saved 
with old ones to avoid issues.

> +
> +static void
> +read_uniforms(struct blob_reader *metadata, struct gl_shader_program *prog)
> +{
> +   struct gl_uniform_storage *uniforms;
> +   union gl_constant_value *data;
> +
> +   prog->SamplersValidated = blob_read_uint32(metadata);
> +   prog->data->NumUniformStorage = blob_read_uint32(metadata);
> +   prog->data->NumUniformDataSlots = blob_read_uint32(metadata);
> +
> +   uniforms = rzalloc_array(prog, struct gl_uniform_storage,
> +                            prog->data->NumUniformStorage);
> +   prog->data->UniformStorage = uniforms;
> +
> +   data = rzalloc_array(uniforms, union gl_constant_value,
> +                        prog->data->NumUniformDataSlots);
> +   prog->data->UniformDataSlots = data;
> +
> +   prog->UniformHash = new string_to_uint_map;
> +
> +   for (unsigned i = 0; i < prog->data->NumUniformStorage; i++) {
> +      uniforms[i].type = decode_type_from_blob(metadata);
> +      uniforms[i].array_elements = blob_read_uint32(metadata);
> +      uniforms[i].name = ralloc_strdup(prog, blob_read_string (metadata));
> +      uniforms[i].storage = data + blob_read_uint32(metadata);
> +      uniforms[i].remap_location = blob_read_uint32(metadata);
> +      uniforms[i].block_index = blob_read_uint32(metadata);
> +      uniforms[i].atomic_buffer_index = blob_read_uint32(metadata);
> +      uniforms[i].offset = blob_read_uint32(metadata);
> +      uniforms[i].array_stride = blob_read_uint32(metadata);
> +      uniforms[i].matrix_stride = blob_read_uint32(metadata);
> +      uniforms[i].row_major = blob_read_uint32(metadata);
> +      uniforms[i].num_compatible_subroutines = blob_read_uint32(metadata);
> +      uniforms[i].top_level_array_size = blob_read_uint32(metadata);
> +      uniforms[i].top_level_array_stride = blob_read_uint32(metadata);
> +      prog->UniformHash->put(i, uniforms[i].name);
> +   }
> +}
> +

// Tapani