[Mesa-dev] [wip 2/9] glsl: serialize methods for IR instructions

[Paul Berry]

On 2 January 2014 03:58, Tapani Pälli <tapani.palli at intel.com> wrote:

> diff --git a/src/glsl/ir_serialize.cpp b/src/glsl/ir_serialize.cpp
> new file mode 100644
> index 0000000..30ca018
> --- /dev/null
> +++ b/src/glsl/ir_serialize.cpp
> @@ -0,0 +1,392 @@
> +/* -*- 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_serialize.h"
> +
> +
> +/**
> + * Wraps serialization of an ir instruction, writes ir_type
> + * and length of each instruction package as a header for it
> + */
> +void
> +ir_instruction::serialize(memory_writer &mem)
> +{
> +   uint32_t data_len = 0;
> +   uint8_t ir_type = this->ir_type;
> +   mem.write_uint8_t(ir_type);
> +
> +   int32_t start_pos = mem.position();
> +   mem.write_uint32_t(data_len);
> +
> +   this->serialize_data(mem);
> +
> +   data_len = mem.position() - start_pos - sizeof(data_len);
> +   mem.overwrite(&data_len, sizeof(data_len), start_pos);
>

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).


> +}
> +
> +
> +
> +
> +static void
> +serialize_glsl_type(const glsl_type *type, memory_writer &mem)
>

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
http://lists.freedesktop.org/archives/mesa-dev/2013-November/047740.html).
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.

With those two issues addressed, the patch is:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140113/f6eea161/attachment-0001.html>