[Mesa-dev] [PATCH v5 1/5] genxml: New generated header genX_bits.h (v5)
Dylan Baker
dylan at pnwbakers.com
Sat Mar 25 16:01:50 UTC 2017
Quoting Chad Versace (2017-03-24 19:28:23)
> genX_bits.h contains the sizes of bitfields in genxml instructions,
> structures, and registers. It also defines some functions to query those
> sizes.
>
> isl_surf_init() will use the new header to validate that requested
> pitches fit in their destination bitfields.
>
> What's currently in genX_bits.h:
>
> - Each CONTAINER::Field from gen*.xml that has a bitsize has a macro
> in genX_bits.h:
>
> #define GEN{N}_CONTAINER_Field_bits {bitsize}
>
> - For each set of macros whose name, after stripping the GEN prefix,
> is the same, genX_bits.h contains a query function:
>
> static inline uint32_t __attribute__((pure))
> CONTAINER_Field_bits(const struct gen_device_info *devinfo);
>
> v2 (Chad Versace):
> - Parse the XML instead of scraping the generated gen*_pack.h headers.
>
> v3 (Dylan Baker):
> - Port to Mako.
>
> v4 (Jason Ekstrand):
> - Make the _bits functions take a gen_device_info.
>
> v5 (Chad Versace):
> - Fix autotools out-of-tree build.
> - Fix Android build. Tested with git://github.com/android-ia/manifest.
> - Fix macro names. They were all missing the "_bits" suffix.
> - Fix macros names more. Remove all double-underscores.
> - Unindent all generated code. (It was floating in a sea of whitespace).
I really think this is a bad design decision, and feels rather like the tail
wagging the dog. You're sacrificing the readability and editability of the code
that humans are actually supposed to edit, to gain something that the compiler
doesn't care about (the most common path), and the indent command can solve in
the cases where one wants to carefully read the generated code.
> - Reformat header to appear human-written not machine-generated.
> - Sort gens from high to low. Newest gens should come first because,
> when we read code, we likely want to read the gen8/9 code and ignore
> the gen4 code. So put the gen4 code at the bottom.
> - Replace 'const' attributes with 'pure', because the functions now
> have a pointer parameter.
> - Add --cpp-guard flag. Used by Android.
> - Kill class FieldCollection. After Jason's rewrite, it was just
> a dict.
>
> Co-authored-by: Dylan Baker <dylan at pnwbakers.com>
> Co-authored-by: Jason Ekstrand <jason at jlekstrand.net>
> ---
> src/intel/Android.genxml.mk | 9 +-
> src/intel/Makefile.genxml.am | 6 +-
> src/intel/Makefile.sources | 6 +-
> src/intel/genxml/.gitignore | 1 +
> src/intel/genxml/gen_bits_header.py | 281 ++++++++++++++++++++++++++++++++++++
> 5 files changed, 300 insertions(+), 3 deletions(-)
> create mode 100644 src/intel/genxml/gen_bits_header.py
>
> diff --git a/src/intel/Android.genxml.mk b/src/intel/Android.genxml.mk
> index 79de7843801..842d0e13a33 100644
> --- a/src/intel/Android.genxml.mk
> +++ b/src/intel/Android.genxml.mk
> @@ -46,9 +46,16 @@ LOCAL_GENERATED_SOURCES += $(addprefix $(intermediates)/, $(GENXML_GENERATED_FIL
> define header-gen
> @mkdir -p $(dir $@)
> @echo "Gen Header: $(PRIVATE_MODULE) <= $(notdir $(@))"
> - $(hide) $(PRIVATE_SCRIPT) $(PRIVATE_XML) > $@
> + $(hide) $(PRIVATE_SCRIPT) $(PRIVATE_SCRIPT_FLAGS) $(PRIVATE_XML) > $@
> endef
>
> +$(intermediates)/genxml/genX_bits.h: PRIVATE_SCRIPT := $(MESA_PYTHON2) $(LOCAL_PATH)/genxml/gen_bits_header.py
> +$(intermediates)/genxml/genX_bits.h: PRIVATE_SCRIPT_FLAGS := --cpp-guard=GENX_BITS_H
> +$(intermediates)/genxml/genX_bits.h: PRIVATE_XML := $(addprefix $(LOCAL_PATH)/,$(GENXML_XML_FILES))
> +$(intermediates)/genxml/genX_bits.h: $(LOCAL_PATH)/genxml/gen_bits_header.py
> +$(intermediates)/genxml/genX_bits.h: $(addprefix $(LOCAL_PATH)/,$(GENXML_XML_FILES))
> + $(call header-gen)
> +
> $(intermediates)/genxml/gen4_pack.h: PRIVATE_SCRIPT := $(MESA_PYTHON2) $(LOCAL_PATH)/genxml/gen_pack_header.py
> $(intermediates)/genxml/gen4_pack.h: PRIVATE_XML := $(LOCAL_PATH)/genxml/gen4.xml
> $(intermediates)/genxml/gen4_pack.h: $(LOCAL_PATH)/genxml/gen4.xml $(LOCAL_PATH)/genxml/gen_pack_header.py
> diff --git a/src/intel/Makefile.genxml.am b/src/intel/Makefile.genxml.am
> index 01a02b63b44..474b751f5fd 100644
> --- a/src/intel/Makefile.genxml.am
> +++ b/src/intel/Makefile.genxml.am
> @@ -30,7 +30,7 @@ EXTRA_DIST += \
>
> SUFFIXES = _pack.h _xml.h .xml
>
> -$(GENXML_GENERATED_FILES): genxml/gen_pack_header.py
> +$(GENXML_GENERATED_PACK_FILES): genxml/gen_pack_header.py
>
> .xml_pack.h:
> $(MKDIR_GEN)
> @@ -42,6 +42,10 @@ $(AUBINATOR_GENERATED_FILES): genxml/gen_zipped_file.py
> $(MKDIR_GEN)
> $(AM_V_GEN) $(PYTHON2) $(srcdir)/genxml/gen_zipped_file.py $< > $@ || ($(RM) $@; false)
>
> +genxml/genX_bits.h: genxml/gen_bits_header.py $(GENXML_XML_FILES)
> + $(MKDIR_GEN)
> + $(PYTHON_GEN) $< -o $@ $(addprefix $(srcdir)/,$(GENXML_XML_FILES))
> +
> EXTRA_DIST += \
> genxml/genX_pack.h \
> genxml/gen_macros.h \
> diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
> index 88bcf60f6e3..c56891643ce 100644
> --- a/src/intel/Makefile.sources
> +++ b/src/intel/Makefile.sources
> @@ -119,7 +119,7 @@ GENXML_XML_FILES = \
> genxml/gen8.xml \
> genxml/gen9.xml
>
> -GENXML_GENERATED_FILES = \
> +GENXML_GENERATED_PACK_FILES = \
> genxml/gen4_pack.h \
> genxml/gen45_pack.h \
> genxml/gen5_pack.h \
> @@ -129,6 +129,10 @@ GENXML_GENERATED_FILES = \
> genxml/gen8_pack.h \
> genxml/gen9_pack.h
>
> +GENXML_GENERATED_FILES = \
> + $(GENXML_GENERATED_PACK_FILES) \
> + genxml/genX_bits.h
> +
> AUBINATOR_GENERATED_FILES = \
> genxml/gen6_xml.h \
> genxml/gen7_xml.h \
> diff --git a/src/intel/genxml/.gitignore b/src/intel/genxml/.gitignore
> index c5672b5595c..3e2f1cfa9f0 100644
> --- a/src/intel/genxml/.gitignore
> +++ b/src/intel/genxml/.gitignore
> @@ -1,2 +1,3 @@
> +gen*_bits.h
> gen*_pack.h
> gen*_xml.h
> diff --git a/src/intel/genxml/gen_bits_header.py b/src/intel/genxml/gen_bits_header.py
> new file mode 100644
> index 00000000000..a7006457794
> --- /dev/null
> +++ b/src/intel/genxml/gen_bits_header.py
> @@ -0,0 +1,281 @@
> +#encoding=utf-8
> +# Copyright © 2017 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 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.
> +
> +from __future__ import (
> + absolute_import, division, print_function, unicode_literals
> +)
> +
> +import argparse
> +import os
> +import sys
> +import xml.parsers.expat
> +
> +from mako.template import Template
> +
> +TEMPLATE = Template("""\
> +<%!
> +from operator import itemgetter
> +%>\
> +/*
> + * Copyright © 2017 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.
> + */
> +
> +/* THIS FILE HAS BEEN GENERATED, DO NOT HAND EDIT.
> + *
> + * Sizes of bitfields in genxml instructions, structures, and registers.
> + */
> +
> +#ifndef ${guard}
> +#define ${guard}
> +
> +#include <stdint.h>
> +
> +#include "common/gen_device_info.h"
> +#include "util/macros.h"
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +% for _, field in sorted(fields.iteritems(), key=itemgetter(0)):
> +
> +/* ${field.container_name}::${field.name} */
> +% for gen, bits in sorted(field.bits_by_gen.iteritems(), reverse=True):
> +#define ${gen.prefix(field.token_name, padded=True)} ${bits}
> +% endfor
> +
> +static inline uint32_t ATTRIBUTE_PURE
> +${field.token_name}(const struct gen_device_info *devinfo)
> +{
> + switch (devinfo->gen) {
> + case 9: return ${field.bits(9)};
> + case 8: return ${field.bits(8)};
> + case 7:
> + if (devinfo->is_haswell) {
> + return ${field.bits(7.5)};
> + } else {
> + return ${field.bits(7)};
> + }
> + case 6: return ${field.bits(6)};
> + case 5: return ${field.bits(5)};
> + case 4:
> + if (devinfo->is_g4x) {
> + return ${field.bits(4.5)};
> + } else {
> + return ${field.bits(4)};
> + }
> + default:
> + unreachable("Invalid hardware generation");
> + }
> +}
> +% endfor
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* ${guard} */""", output_encoding='utf-8')
> +
> +def to_alphanum(name):
> + substitutions = {
> + ' ': '',
> + '/': '',
> + '[': '',
> + ']': '',
> + '(': '',
> + ')': '',
> + '-': '',
> + ':': '',
> + '.': '',
> + ',': '',
> + '=': '',
> + '>': '',
> + '#': '',
> + 'α': 'alpha',
> + '&': '',
> + '*': '',
> + '"': '',
> + '+': '',
> + '\'': '',
> + }
> +
> + for i, j in substitutions.items():
> + name = name.replace(i, j)
> +
> + return name
> +
> +def safe_name(name):
> + name = to_alphanum(name)
> + if not name[0].isalpha():
> + name = '_' + name
> + return name
> +
> +class Gen(object):
> +
> + def __init__(self, z):
> + # Convert potential "major.minor" string
> + z = float(z)
> + if z < 10:
> + z *= 10
> + self.tenx = int(z)
> +
> + def __lt__(self, other):
> + return self.tenx < other.tenx
> +
> + def __hash__(self):
> + return hash(self.tenx)
> +
> + def __eq__(self, other):
> + return self.tenx == other.tenx
> +
> + def prefix(self, token, padded=False):
> + gen = self.tenx
> + pad = ''
> +
> + if gen % 10 == 0:
> + gen //= 10
> + if padded:
> + pad = ' '
> +
> + if token[0] == '_':
> + token = token[1:]
> +
> + return 'GEN{}_{}{}'.format(gen, token, pad)
> +
> +class Field(object):
> +
> + def __init__(self, container_name, name):
> + self.container_name = container_name
> + self.name = name
> + self.token_name = safe_name('_'.join([self.container_name, self.name, 'bits']))
> + self.bits_by_gen = {}
> +
> + def add_gen(self, gen, xml_attrs):
> + assert isinstance(gen, Gen)
> + start = int(xml_attrs['start'])
> + end = int(xml_attrs['end'])
> + self.bits_by_gen[gen] = 1 + end - start
> +
> + def bits(self, gen):
> + if not isinstance(gen, Gen):
> + gen = Gen(gen)
> + return self.bits_by_gen.get(gen, 0)
> +
> +class XmlParser(object):
> +
> + def __init__(self, fields):
> + self.parser = xml.parsers.expat.ParserCreate()
> + self.parser.StartElementHandler = self.start_element
> + self.parser.EndElementHandler = self.end_element
> +
> + self.gen = None
> + self.container_name = None
> + self.fields = fields
> +
> + def parse(self, filename):
> + with open(filename) as f:
> + self.parser.ParseFile(f)
> +
> + def start_element(self, name, attrs):
> + if name == 'genxml':
> + self.gen = Gen(attrs['gen'])
> + elif name in ('instruction', 'struct', 'register'):
> + self.start_container(attrs)
> + elif name == 'field':
> + self.start_field(attrs)
> + else:
> + pass
> +
> + def end_element(self, name):
> + if name == 'genxml':
> + self.gen = None
> + elif name in ('instruction', 'struct', 'register'):
> + self.container_name = None
> + else:
> + pass
else: pass is a bit of a pointless statement
> +
> + def start_container(self, attrs):
> + assert self.container_name is None
> + self.container_name = attrs['name']
> +
> + def start_field(self, attrs):
> + if self.container_name is None:
> + return
> +
> + field_name = attrs.get('name', None)
> + if not field_name:
> + return
> +
> + key = (self.container_name, field_name)
> + if key not in self.fields.keys():
key key not in self.feilds
Which does the same thing, but doesn't build an intermediate list to check.
> + self.fields[key] = Field(self.container_name, field_name)
> + self.fields[key].add_gen(self.gen, attrs)
> +
> +def parse_args():
> + p = argparse.ArgumentParser()
> + p.add_argument('-o', '--output', type=str,
> + help="If OUTPUT is unset or '-', then it defaults to '/dev/stdout'")
> + p.add_argument('--cpp-guard', type=str,
> + help='If unset, then CPP_GUARD is derived from OUTPUT.')
> + p.add_argument('xml_sources', metavar='XML_SOURCE', nargs='+')
> +
> + pargs = p.parse_args()
> +
> + if pargs.output in (None, '-'):
> + pargs.output = '/dev/stdout'
This is just a suggestion, feel free to ignore
I forgot to do something here, but this is one case where using
argparse.FileType might actually be the right thing to do.
it would like like:
p.add_argument('-o', '--output', type=argparse.FileType('wb'), default=sys.stdout, ...)
Then you could drop the hunk above and the 'with open(...)' in the main
function. It's also more portable.
Dylan
> +
> + if pargs.cpp_guard is None:
> + pargs.cpp_guard = os.path.basename(pargs.output).upper().replace('.', '_')
> +
> + return pargs
> +
> +def main():
> + pargs = parse_args()
> +
> + # Maps (container_name, field_name) => Field
> + fields = {}
> +
> + for source in pargs.xml_sources:
> + XmlParser(fields).parse(source)
> +
> + with open(pargs.output, 'wb') as f:
> + f.write(TEMPLATE.render(fields=fields, guard=pargs.cpp_guard))
> +
> +if __name__ == '__main__':
> + main()
> --
> 2.12.0
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170325/a8eb7354/attachment.sig>
More information about the mesa-dev
mailing list