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

[Tapani Pälli]

On 01/13/2014 11:58 PM, Paul Berry wrote:
> On 2 January 2014 03:58, Tapani Pälli <tapani.palli at intel.com 
> <mailto: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).

I left error checking away from serialization but I left it there in the 
memory_writer class to be available if some other possible user would 
want it but it can be removed.

>     +}
>     +
>     +
>     +
>     +
>     +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.
>

I liked the idea but I did not manage to come up with a good solution to 
iterate through builtin types at that time except hardcoding. One 
possible way would be to extend glsl_symbol_table so that one could 
iterate through types and come up with method to recognize that type is 
a builtin type, I will try to get it working from there. Currently only 
samplers are skipped, I took the check against name away as you 
mentioned that interfaces might have same exact name but different content.


> With those two issues addressed, the patch is:
>
> Reviewed-by: Paul Berry <stereotype441 at gmail.com 
> <mailto:stereotype441 at gmail.com>>

// Tapani

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140114/5a025ba2/attachment.html>