On 2 January 2014 03:58, Tapani Pälli <tapani.pa...@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 <stereotype...@gmail.com>
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev