[Mesa-dev] [PATCH] intel: genxml: prevent missing ; with address fields dwords

[Jason Ekstrand]

On Mon, Mar 13, 2017 at 3:46 AM, Lionel Landwerlin <
lionel.g.landwerlin at intel.com> wrote:

> Before this change, the generator could print this kind of things :
>
>    const uint32_t v0 =
>       __gen_uint(values->ValidBit, 0, 0) |
>       __gen_uint(values->FaultType, 1, 2) |
>       __gen_uint(values->SRCIDofFault, 3, 10) |
>       __gen_uint(values->GTTSEL, 11, 1) |
>    dw[0] = __gen_combine_address(data, &dw[0],
> values->VirtualAddressofFault, v0);
>
> This change fix the trailing '|'.
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> ---
>  src/intel/genxml/gen_pack_header.py | 54 ++++++++++++++++++------------
> -------
>  1 file changed, 26 insertions(+), 28 deletions(-)
>
> diff --git a/src/intel/genxml/gen_pack_header.py
> b/src/intel/genxml/gen_pack_header.py
> index a1befaf45e..88a42e4b57 100644
> --- a/src/intel/genxml/gen_pack_header.py
> +++ b/src/intel/genxml/gen_pack_header.py
> @@ -415,51 +415,49 @@ class Group(object):
>                  v = "0"
>
>              field_index = 0
> +            non_address_fields = []
>              for field in dw.fields:
>                  if field.type != "mbo":
>                      name = field.name + field.dim
>
>                  if field.type == "mbo":
> -                    s = "__gen_mbo(%d, %d)" % \
> -                        (field.start - dword_start, field.end -
> dword_start)
> +                    non_address_fields.append("__gen_mbo(%d, %d)" % \
> +                        (field.start - dword_start, field.end -
> dword_start))
>                  elif field.type == "address":
> -                    s = None
> +                    pass
>                  elif field.type == "uint":
> -                    s = "__gen_uint(values->%s, %d, %d)" % \
> -                        (name, field.start - dword_start, field.end -
> dword_start)
> +                    non_address_fields.append("__gen_uint(values->%s,
> %d, %d)" % \
> +                        (name, field.start - dword_start, field.end -
> dword_start))
>                  elif field.type in self.parser.enums:
> -                    s = "__gen_uint(values->%s, %d, %d)" % \
> -                        (name, field.start - dword_start, field.end -
> dword_start)
> +                    non_address_fields.append("__gen_uint(values->%s,
> %d, %d)" % \
> +                        (name, field.start - dword_start, field.end -
> dword_start))
>                  elif field.type == "int":
> -                    s = "__gen_sint(values->%s, %d, %d)" % \
> -                        (name, field.start - dword_start, field.end -
> dword_start)
> +                    non_address_fields.append("__gen_sint(values->%s,
> %d, %d)" % \
> +                        (name, field.start - dword_start, field.end -
> dword_start))
>                  elif field.type == "bool":
> -                    s = "__gen_uint(values->%s, %d, %d)" % \
> -                        (name, field.start - dword_start, field.end -
> dword_start)
> +                    non_address_fields.append("__gen_uint(values->%s,
> %d, %d)" % \
> +                        (name, field.start - dword_start, field.end -
> dword_start))
>                  elif field.type == "float":
> -                    s = "__gen_float(values->%s)" % name
> +                    non_address_fields.append("__gen_float(values->%s)"
> % name)
>                  elif field.type == "offset":
> -                    s = "__gen_offset(values->%s, %d, %d)" % \
> -                        (name, field.start - dword_start, field.end -
> dword_start)
> +                    non_address_fields.append("__gen_offset(values->%s,
> %d, %d)" % \
> +                        (name, field.start - dword_start, field.end -
> dword_start))
>                  elif field.type == 'ufixed':
> -                    s = "__gen_ufixed(values->%s, %d, %d, %d)" % \
> -                        (name, field.start - dword_start, field.end -
> dword_start, field.fractional_size)
> +                    non_address_fields.append("__gen_ufixed(values->%s,
> %d, %d, %d)" % \
> +                        (name, field.start - dword_start, field.end -
> dword_start, field.fractional_size))
>                  elif field.type == 'sfixed':
> -                    s = "__gen_sfixed(values->%s, %d, %d, %d)" % \
> -                        (name, field.start - dword_start, field.end -
> dword_start, field.fractional_size)
> +                    non_address_fields.append("__gen_sfixed(values->%s,
> %d, %d, %d)" % \
> +                        (name, field.start - dword_start, field.end -
> dword_start, field.fractional_size))
>                  elif field.type in self.parser.structs:
> -                    s = "__gen_uint(v%d_%d, %d, %d)" % \
> -                        (index, field_index, field.start - dword_start,
> field.end - dword_start)
> +                    non_address_fields.append("__gen_uint(v%d_%d, %d,
> %d)" % \
> +                        (index, field_index, field.start - dword_start,
> field.end - dword_start))
>                      field_index = field_index + 1
>                  else:
> -                    print("/* unhandled field %s, type %s */\n" % (name,
> field.type))
> -                    s = None
> -
> -                if not s == None:
> -                    if field == dw.fields[-1]:
> -                        print("      %s;" % s)
> -                    else:
> -                        print("      %s |" % s)
> +                    non_address_fields.append("/* unhandled field %s,
> type %s */\n" % \
> +                                              (name, field.type))
> +
> +            if len(non_address_fields) > 0:
> +                print("%s;" % " |\n".join(map(lambda x: "      %s" % x,
> non_address_fields)))
>

Another way to do this (which I find easier to read) would be

print(" |\n".join("      " + f for f in non_address_fields) + ";")

Gets rid of the lambda and some of the string formatting.  Either way,

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

We really need to clean up the generator. :-/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170313/dbdf1475/attachment-0001.html>