[Xcb] [PATCH libxcb 4/6] generator: sumof: support any type, generate explicit code

[Ran Benita]

On Wed, Sep 03, 2014 at 01:22:39PM +0200, Christian Linhart wrote:
> A sumof-expression now generates explicit code ( for-loop etc )
> instead of calling xcb_sumof.
> 
> This way, it supports any type which can be added.
> Previously, only uint_8 was supported.
> 
> Here's an example and the generated code:
> 
> xml:
> <struct name="SumofTest">
>    <field type="CARD32" name="len" />
>    <list type="CARD16" name="mylist1">
> 	   <fieldref>len</fieldref>
>    </list>
>    <list type="CARD8" name="mylist2">
> 	   <sumof ref="mylist1"/>
>    </list>
> </struct>
> 
> declaration of tempvars at the start of enclosing function:
>     int xcb_pre_tmp_1; /* sumof length */
>     int xcb_pre_tmp_2; /* sumof loop counter */
>     int64_t xcb_pre_tmp_3; /* sumof sum */
>     const uint16_t* xcb_pre_tmp_4; /* sumof list ptr */
> 
> code:
>     /* mylist2 */
>     /* sumof start */
>     xcb_pre_tmp_1 = _aux->len;
>     xcb_pre_tmp_3 = 0;
>     xcb_pre_tmp_4 = xcb_input_sumof_test_mylist_1(_aux);
>     for ( xcb_pre_tmp_2 = 0; xcb_pre_tmp_2 < xcb_pre_tmp_1; xcb_pre_tmp_2++) {
>         xcb_pre_tmp_3 += *xcb_pre_tmp_4;
>         xcb_pre_tmp_4++;
>     }
>     /* sumof end. Result is in xcb_pre_tmp_3 */
>     xcb_block_len += xcb_pre_tmp_3 * sizeof(uint8_t);
> 
> This patch is also a preparation for sumof which can access
> fields of lists of struct, etc.
> ---
>  src/c_client.py | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/src/c_client.py b/src/c_client.py
> index 193e050..6b694d5 100644
> --- a/src/c_client.py
> +++ b/src/c_client.py
> @@ -1609,15 +1609,42 @@ def _c_accessor_get_expr(expr, field_mapping):
>  
>          if field is None:
>              raise Exception("list field '%s' referenced by sumof not found" % expr.lenfield_name)
>          list_name = field_mapping[field.c_field_name][0]
>          c_length_func = "%s(%s)" % (field.c_length_name, list_name)
>          # note: xcb_sumof() has only been defined for integers

Outdated comment?

>          c_length_func = _c_accessor_get_expr(field.type.expr, field_mapping)
> -        return 'xcb_sumof(%s, %s)' % (list_name, c_length_func)
> +        #create explicit code for computing the sum.
> +        #This works for all C-types which can be added to int64_t with +=

I don't mind the int64_t approach, I'd probably do the same. But I'm
guessing the real type is too hard to come by?

> +        _c_pre.start()
> +        lengthvar = _c_pre.get_tempvarname()
> +        loopvar = _c_pre.get_tempvarname()
> +        sumvar = _c_pre.get_tempvarname()
> +        listvar = _c_pre.get_tempvarname()
> +        _c_pre.tempvar( "int %s; /* sumof length */", lengthvar )
> +        _c_pre.tempvar( "int %s; /* sumof loop counter */", loopvar )
> +        _c_pre.tempvar( "int64_t %s; /* sumof sum */", sumvar )

Have you considered, instead of random fresh var + comments,
incorporating a descriptive name to the tempvarname?
The generated code as-is is readable enough though.

> +        _c_pre.tempvar(
> +             "const %s* %s; /* sumof list ptr */", field.c_field_type, listvar )

No need to line break here!

> +        _c_pre.code( "/* sumof start */" )
> +        _c_pre.code( "%s = %s;", lengthvar, c_length_func )
> +        _c_pre.code( "%s = 0;", sumvar )
> +        _c_pre.code( "%s = %s;", listvar, list_name )
> +        _c_pre.code(
> +            "for ( %s = 0; %s < %s; %s++) {",

Here too! (the string line).

> +           loopvar, loopvar, lengthvar, loopvar )
> +        _c_pre.indent()
> +        _c_pre.code( "%s += *%s;", sumvar, listvar )
> +        _c_pre.code( "%s++;", listvar );
> +        _c_pre.pop_indent()
> +        _c_pre.code( "}" )
> +        _c_pre.code( "/* sumof end. Result is in %s */", sumvar )
> +        _c_pre.end()
> +        return sumvar;

Semicolon :)

> +        #return 'xcb_sumof(%s, %s)' % (list_name, c_length_func)

I think you should just delete this line.

>      elif expr.op != None:
>          return ('(' + _c_accessor_get_expr(expr.lhs, field_mapping) +
>                  ' ' + expr.op + ' ' +
>                  _c_accessor_get_expr(expr.rhs, field_mapping) + ')')
>      elif expr.bitfield:
>          return 'xcb_popcount(' + lenexp + ')'
>      else:
> -- 
> 2.0.1
> 
> _______________________________________________
> Xcb mailing list
> Xcb at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xcb